8215948: [TESTBUG] gtest pseudo-JavaThreads could be more regular JavaThreads

Reviewed-by: coleenp, pchilanomate

Author: David Holmes

test/hotspot/gtest/threadHelper.inline.hpp

@@ -26,11 +26,29 @@
 
 #include "runtime/mutex.hpp"
 #include "runtime/semaphore.hpp"
+#include "runtime/interfaceSupport.inline.hpp"
 #include "runtime/thread.inline.hpp"
 #include "runtime/vmThread.hpp"
 #include "runtime/vmOperations.hpp"
 #include "unittest.hpp"
 
+static void startTestThread(JavaThread* thread, const char* name) {
+  EXCEPTION_MARK;
+  HandleMark hm(THREAD);
+  Handle thread_oop;
+
+  // This code can be called from the main thread, which is _thread_in_native,
+  // or by an existing JavaTestThread, which is _thread_in_vm.
+  if (THREAD->thread_state() == _thread_in_native) {
+    ThreadInVMfromNative tivfn(THREAD);
+    thread_oop = JavaThread::create_system_thread_object(name, false /* not visible */, CHECK);
+    JavaThread::start_internal_daemon(THREAD, thread, thread_oop, NoPriority);
+  } else {
+    thread_oop = JavaThread::create_system_thread_object(name, false /* not visible */, CHECK);
+    JavaThread::start_internal_daemon(THREAD, thread, thread_oop, NoPriority);
+  }
+}
+
 class VM_StopSafepoint : public VM_Operation {
 public:
   Semaphore* _running;
@@ -44,38 +62,29 @@ class VM_StopSafepoint : public VM_Operation {
 
 // This class and thread keep the non-safepoint op running while we do our testing.
 class VMThreadBlocker : public JavaThread {
-public:
   Semaphore _ready;
   Semaphore _unblock;
-  VMThreadBlocker() {}
-  virtual ~VMThreadBlocker() {}
-  const char* get_thread_name_string(char* buf, int buflen) const {
-    return "VMThreadBlocker";
-  }
-  void run() {
-    this->set_thread_state(_thread_in_vm);
-    {
-      MutexLocker ml(Threads_lock);
-      Threads::add(this);
-    }
-    VM_StopSafepoint ss(&_ready, &_unblock);
+
+  static void blocker_thread_entry(JavaThread* thread, TRAPS) {
+    VMThreadBlocker* t = static_cast<VMThreadBlocker*>(thread);
+    VM_StopSafepoint ss(&t->_ready, &t->_unblock);
     VMThread::execute(&ss);
   }
 
-  // Override as JavaThread::post_run() calls JavaThread::exit which
-  // expects a valid thread object oop.
-  virtual void post_run() {
-    Threads::remove(this, false);
-    this->smr_delete();
-  }
+  VMThreadBlocker() : JavaThread(&blocker_thread_entry) {};
 
-  void doit() {
-    if (os::create_thread(this, os::os_thread)) {
-      os::start_thread(this);
-    } else {
-      ASSERT_TRUE(false);
-    }
+  virtual ~VMThreadBlocker() {}
+
+public:
+  // Convenience method for client code
+  static VMThreadBlocker* start() {
+    const char* name = "VMThreadBlocker";
+    VMThreadBlocker* thread = new VMThreadBlocker();
+    JavaThread::vm_exit_on_osthread_failure(thread);
+    startTestThread(thread, name);
+    return thread;
   }
+
   void ready() {
     _ready.wait();
   }
@@ -86,52 +95,34 @@ class VMThreadBlocker : public JavaThread {
 
 // For testing in a real JavaThread.
 class JavaTestThread : public JavaThread {
-public:
   Semaphore* _post;
+
+protected:
   JavaTestThread(Semaphore* post)
-    : _post(post) {
+    : JavaThread(&test_thread_entry), _post(post) {
+    JavaThread::vm_exit_on_osthread_failure(this);
   }
   virtual ~JavaTestThread() {}
 
-  const char* get_thread_name_string(char* buf, int buflen) const {
-    return "JavaTestThread";
-  }
-
-  void pre_run() {
-    this->set_thread_state(_thread_in_vm);
-    {
-      MutexLocker ml(Threads_lock);
-      Threads::add(this);
-    }
+public:
+  // simplified starting for callers and subclasses
+  void doit() {
+    startTestThread(this, "JavaTestThread");
   }
 
   virtual void main_run() = 0;
 
-  void run() {
-    main_run();
-  }
-
-  // Override as JavaThread::post_run() calls JavaThread::exit which
-  // expects a valid thread object oop. And we need to call signal.
-  void post_run() {
-    Threads::remove(this, false);
-    _post->signal();
-    this->smr_delete();
-  }
-
-  void doit() {
-    if (os::create_thread(this, os::os_thread)) {
-      os::start_thread(this);
-    } else {
-      ASSERT_TRUE(false);
-    }
+  static void test_thread_entry(JavaThread* thread, TRAPS) {
+    JavaTestThread* t = static_cast<JavaTestThread*>(thread);
+    t->main_run();
+    t->_post->signal();
   }
 };
 
 template <typename FUNC>
 class SingleTestThread : public JavaTestThread {
-public:
   FUNC& _f;
+public:
   SingleTestThread(Semaphore* post, FUNC& f)
     : JavaTestThread(post), _f(f) {
   }
@@ -147,8 +138,8 @@ template <typename TESTFUNC>
 static void nomt_test_doer(TESTFUNC &f) {
   Semaphore post;
 
-  VMThreadBlocker* blocker = new VMThreadBlocker();
-  blocker->doit();
+  VMThreadBlocker* blocker = VMThreadBlocker::start();
+
   blocker->ready();
 
   SingleTestThread<TESTFUNC>* stt = new SingleTestThread<TESTFUNC>(&post, f);
@@ -162,8 +153,8 @@ template <typename RUNNER>
 static void mt_test_doer() {
   Semaphore post;
 
-  VMThreadBlocker* blocker = new VMThreadBlocker();
-  blocker->doit();
+  VMThreadBlocker* blocker = VMThreadBlocker::start();
+
   blocker->ready();
 
   RUNNER* runner = new RUNNER(&post);

test/hotspot/gtest/utilities/test_concurrentHashtable.cpp

@@ -1065,15 +1065,23 @@ TEST_VM(ConcurrentHashTable, concurrent_get_insert_bulk_delete) {
 
 class MT_BD_Thread : public JavaTestThread {
   TestTable::BulkDeleteTask* _bd;
+  Semaphore run;
+
   public:
-  MT_BD_Thread(Semaphore* post, TestTable::BulkDeleteTask* bd)
-    : JavaTestThread(post), _bd(bd){}
+  MT_BD_Thread(Semaphore* post)
+    : JavaTestThread(post) {}
   virtual ~MT_BD_Thread() {}
   void main_run() {
+    run.wait();
     MyDel del;
     while(_bd->do_task(this, *this, del));
   }
 
+  void set_bd_task(TestTable::BulkDeleteTask* bd) {
+    _bd = bd;
+    run.signal();
+  }
+
   bool operator()(uintptr_t* val) {
     return true;
   }
@@ -1098,15 +1106,21 @@ class Driver_BD_Thread : public JavaTestThread {
       TestLookup tl(v);
       EXPECT_TRUE(cht->insert(this, tl, v)) << "Inserting an unique value should work.";
     }
-    TestTable::BulkDeleteTask bdt(cht, true /* mt */ );
-    EXPECT_TRUE(bdt.prepare(this)) << "Uncontended prepare must work.";
 
+    // Must create and start threads before acquiring mutex inside BulkDeleteTask.
     MT_BD_Thread* tt[4];
     for (int i = 0; i < 4; i++) {
-      tt[i] = new MT_BD_Thread(&done, &bdt);
+      tt[i] = new MT_BD_Thread(&done);
       tt[i]->doit();
     }
 
+    TestTable::BulkDeleteTask bdt(cht, true /* mt */ );
+    EXPECT_TRUE(bdt.prepare(this)) << "Uncontended prepare must work.";
+
+    for (int i = 0; i < 4; i++) {
+      tt[i]->set_bd_task(&bdt);
+    }
+
     for (uintptr_t v = 1; v < 99999; v++ ) {
       TestLookup tl(v);
       cht_get_copy(cht, this, tl);