8180599: Possibly miss to iterate monitors on thread exit
authorrkennke
Fri, 19 May 2017 12:14:38 +0200
changeset 46484 688e3a206b86
parent 46483 94ffd2984ec9
child 46485 f10c8f2b4651
8180599: Possibly miss to iterate monitors on thread exit Summary: Move thread-local monitors to global lists before thread is removed from global threads list, to ensure all monitors get scanned Reviewed-by: dholmes, rehn
hotspot/src/share/vm/runtime/synchronizer.cpp
hotspot/src/share/vm/runtime/thread.cpp
--- a/hotspot/src/share/vm/runtime/synchronizer.cpp	Fri May 19 06:50:58 2017 +0000
+++ b/hotspot/src/share/vm/runtime/synchronizer.cpp	Fri May 19 12:14:38 2017 +0200
@@ -1283,14 +1283,14 @@
 // a global gOmInUseList under the global list lock so these
 // will continue to be scanned.
 //
-// We currently call omFlush() from the Thread:: dtor _after the thread
+// We currently call omFlush() from Threads::remove() _before the thread
 // has been excised from the thread list and is no longer a mutator.
-// That means that omFlush() can run concurrently with a safepoint and
-// the scavenge operator.  Calling omFlush() from JavaThread::exit() might
-// be a better choice as we could safely reason that that the JVM is
-// not at a safepoint at the time of the call, and thus there could
-// be not inopportune interleavings between omFlush() and the scavenge
-// operator.
+// This means that omFlush() can not run concurrently with a safepoint and
+// interleave with the scavenge operator. In particular, this ensures that
+// the thread's monitors are scanned by a GC safepoint, either via
+// Thread::oops_do() (if safepoint happens before omFlush()) or via
+// ObjectSynchronizer::oops_do() (if it happens after omFlush() and the thread's
+// monitors have been transferred to the global in-use list).
 
 void ObjectSynchronizer::omFlush(Thread * Self) {
   ObjectMonitor * list = Self->omFreeList;  // Null-terminated SLL
@@ -1338,6 +1338,8 @@
     tail->FreeNext = gFreeList;
     gFreeList = list;
     gMonitorFreeCount += tally;
+    assert(Self->omFreeCount == tally, "free-count off");
+    Self->omFreeCount = 0;
   }
 
   if (inUseTail != NULL) {
--- a/hotspot/src/share/vm/runtime/thread.cpp	Fri May 19 06:50:58 2017 +0000
+++ b/hotspot/src/share/vm/runtime/thread.cpp	Fri May 19 12:14:38 2017 +0200
@@ -336,9 +336,6 @@
 
 
 Thread::~Thread() {
-  // Reclaim the objectmonitors from the omFreeList of the moribund thread.
-  ObjectSynchronizer::omFlush(this);
-
   EVENT_THREAD_DESTRUCT(this);
 
   // stack_base can be NULL if the thread is never started or exited before
@@ -4251,6 +4248,10 @@
 }
 
 void Threads::remove(JavaThread* p) {
+
+  // Reclaim the objectmonitors from the omInUseList and omFreeList of the moribund thread.
+  ObjectSynchronizer::omFlush(p);
+
   // Extra scope needed for Thread_lock, so we can check
   // that we do not remove thread without safepoint code notice
   { MutexLocker ml(Threads_lock);