OpenJDK / jdk / jdk
changeset 46484:688e3a206b86
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
| author | rkennke |
|---|---|
| date | Fri, 19 May 2017 12:14:38 +0200 |
| parents | 94ffd2984ec9 |
| children | f10c8f2b4651 |
| files | hotspot/src/share/vm/runtime/synchronizer.cpp hotspot/src/share/vm/runtime/thread.cpp |
| diffstat | 2 files changed, 13 insertions(+), 10 deletions(-) [+] |
line wrap: on
line diff
--- 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);