From 4297fa04b3f7e62f7e14db8b21f829ffc5c8bd9c Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Mon, 24 Aug 2009 17:51:31 -0600 Subject: [PATCH] run java finalizers in a separate thread to guarantee no application locks are held when doing so --- src/builtin.cpp | 13 +---- src/machine.cpp | 113 ++++++++++++++++++++++++++++++++++--------- src/machine.h | 34 +++++++++++-- test/Finalizers.java | 18 +++++-- 4 files changed, 135 insertions(+), 43 deletions(-) diff --git a/src/builtin.cpp b/src/builtin.cpp index a045d8d451..46e2850c0e 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -909,18 +909,7 @@ Avian_java_lang_Thread_setDaemon object thread = reinterpret_cast(arguments[0]); bool daemon = arguments[1] != 0; - ACQUIRE_RAW(t, t->m->stateLock); - - threadDaemon(t, thread) = daemon; - - if (daemon) { - ++ t->m->daemonCount; - } else { - expect(t, t->m->daemonCount); - -- t->m->daemonCount; - } - - t->m->stateLock->notifyAll(t->systemThread); + setDaemon(t, thread, daemon); } extern "C" JNIEXPORT int64_t JNICALL diff --git a/src/machine.cpp b/src/machine.cpp index 1c8e985548..46e7bf6d9d 100644 --- a/src/machine.cpp +++ b/src/machine.cpp @@ -211,6 +211,23 @@ killZombies(Thread* t, Thread* o) } } +object +makeJavaThread(Thread* t, Thread* parent) +{ + object group; + if (parent) { + group = threadGroup(t, parent->javaThread); + } else { + group = makeThreadGroup(t, 0, 0); + } + + const unsigned NewState = 0; + const unsigned NormalPriority = 5; + + return makeThread + (t, 0, 0, 0, NewState, NormalPriority, 0, 0, 0, t->m->loader, 0, 0, group); +} + unsigned footprint(Thread* t) { @@ -547,12 +564,6 @@ postCollect(Thread* t) void finalizeObject(Thread* t, object o) { - if (t->state == Thread::ExitState) { - // don't waste time running Java finalizers if we're exiting the - // VM - return; - } - for (object c = objectClass(t, o); c; c = classSuper(t, c)) { for (unsigned i = 0; i < arrayLength(t, classMethodTable(t, c)); ++i) { object m = arrayBody(t, classMethodTable(t, c), i); @@ -2016,6 +2027,7 @@ Machine::Machine(System* system, Heap* heap, Finder* finder, processor(processor), rootThread(0), exclusive(0), + finalizeThread(0), jniReferences(0), properties(properties), propertyCount(propertyCount), @@ -2044,6 +2056,7 @@ Machine::Machine(System* system, Heap* heap, Finder* finder, weakReferences(0), tenuredWeakReferences(0), shutdownHooks(0), + objectsToFinalize(0), unsafe(false), triedBuiltinOnLoad(false), heapPoolIndex(0) @@ -2172,23 +2185,11 @@ Thread::init() parent->child = this; } - if (javaThread) { - threadPeer(this, javaThread) = reinterpret_cast(this); - } else { - object group; - if (parent) { - group = threadGroup(this, parent->javaThread); - } else { - group = makeThreadGroup(this, 0, 0); - } - - const unsigned NewState = 0; - const unsigned NormalPriority = 5; - - this->javaThread = makeThread - (this, reinterpret_cast(this), 0, 0, NewState, NormalPriority, - 0, 0, 0, m->loader, 0, 0, group); + if (javaThread == 0) { + this->javaThread = makeJavaThread(this, parent); } + + threadPeer(this, javaThread) = reinterpret_cast(this); } void @@ -2256,6 +2257,22 @@ shutDown(Thread* t) } } } + + // tell finalize thread to exit and wait for it to do so + { ACQUIRE(t, t->m->stateLock); + Thread* finalizeThread = t->m->finalizeThread; + if (finalizeThread) { + t->m->finalizeThread = 0; + t->m->stateLock->notifyAll(t->systemThread); + + while (finalizeThread->state != Thread::ZombieState + and finalizeThread->state != Thread::JoinedState) + { + ENTER(t, Thread::IdleState); + t->m->stateLock->wait(t->systemThread, 0); + } + } + } } void @@ -2519,7 +2536,7 @@ makeNewGeneral(Thread* t, object class_) } if (classVmFlags(t, class_) & HasFinalizerFlag) { - addFinalizer(t, instance, finalizeObject); + addFinalizer(t, instance, 0); } return instance; @@ -3429,7 +3446,24 @@ collect(Thread* t, Heap::CollectionType type) for (; f; f = finalizerNext(t, f)) { void (*function)(Thread*, object); memcpy(&function, &finalizerFinalize(t, f), BytesPerWord); - function(t, finalizerTarget(t, f)); + if (function) { + function(t, finalizerTarget(t, f)); + } else { + m->objectsToFinalize = makePair + (t, finalizerTarget(t, f), m->objectsToFinalize); + } + } + + if (m->objectsToFinalize and m->finalizeThread == 0) { + m->finalizeThread = m->processor->makeThread + (m, makeJavaThread(t, m->rootThread), m->rootThread); + + if (not t->m->system->success + (m->system->start(&(m->finalizeThread->runnable)))) + { + m->finalizeThread->exit(); + m->finalizeThread = 0; + } } } @@ -3502,6 +3536,7 @@ visitRoots(Machine* m, Heap::Visitor* v) v->visit(&(m->types)); v->visit(&(m->jniMethodTable)); v->visit(&(m->shutdownHooks)); + v->visit(&(m->objectsToFinalize)); for (Thread* t = m->rootThread; t; t = t->peer) { ::visitRoots(t, v); @@ -3624,6 +3659,36 @@ runJavaThread(Thread* t) } } +void +runFinalizeThread(Thread* t) +{ + setDaemon(t, t->javaThread, true); + + object list = 0; + PROTECT(t, list); + + while (true) { + { ACQUIRE(t, t->m->stateLock); + + while (t->m->finalizeThread and t->m->objectsToFinalize == 0) { + ENTER(t, Thread::IdleState); + t->m->stateLock->wait(t->systemThread, 0); + } + + if (t->m->finalizeThread == 0) { + return; + } else { + list = t->m->objectsToFinalize; + t->m->objectsToFinalize = 0; + } + } + + for (; list; list = pairSecond(t, list)) { + finalizeObject(t, pairFirst(t, list)); + } + } +} + void noop() { } diff --git a/src/machine.h b/src/machine.h index 078355dbe9..549b464025 100644 --- a/src/machine.h +++ b/src/machine.h @@ -1173,6 +1173,7 @@ class Machine { Processor* processor; Thread* rootThread; Thread* exclusive; + Thread* finalizeThread; Reference* jniReferences; const char** properties; unsigned propertyCount; @@ -1201,6 +1202,7 @@ class Machine { object weakReferences; object tenuredWeakReferences; object shutdownHooks; + object objectsToFinalize; bool unsafe; bool triedBuiltinOnLoad; JavaVMVTable javaVMVTable; @@ -1231,6 +1233,9 @@ inline void stress(Thread* t); void runJavaThread(Thread* t); +void +runFinalizeThread(Thread* t); + class Thread { public: enum State { @@ -1302,10 +1307,14 @@ class Thread { t->m->localThread->set(t); - runJavaThread(t); + if (t == t->m->finalizeThread) { + runFinalizeThread(t); + } else if (t->javaThread) { + runJavaThread(t); - if (t->exception) { - printTrace(t, t->exception); + if (t->exception) { + printTrace(t, t->exception); + } } t->exit(); @@ -2357,6 +2366,25 @@ interrupt(Thread*, Thread* target) target->systemThread->interrupt(); } +inline void +setDaemon(Thread* t, object thread, bool daemon) +{ + ACQUIRE_RAW(t, t->m->stateLock); + + if (threadDaemon(t, thread) != daemon) { + threadDaemon(t, thread) = daemon; + + if (daemon) { + ++ t->m->daemonCount; + } else { + expect(t, t->m->daemonCount); + -- t->m->daemonCount; + } + + t->m->stateLock->notifyAll(t->systemThread); + } +} + object intern(Thread* t, object s); diff --git a/test/Finalizers.java b/test/Finalizers.java index b7844988bf..220a32ba4c 100644 --- a/test/Finalizers.java +++ b/test/Finalizers.java @@ -1,4 +1,5 @@ public class Finalizers { + private static final Object lock = new Object(); private static boolean finalized = false; private static void expect(boolean v) { @@ -6,15 +7,21 @@ public class Finalizers { } protected void finalize() { - finalized = true; + synchronized (lock) { + finalized = true; + lock.notifyAll(); + } } - public static void main(String[] args) { + public static void main(String[] args) throws Exception { new Finalizers(); expect(! finalized); - System.gc(); + synchronized (lock) { + System.gc(); + lock.wait(5000); + } expect(finalized); @@ -24,7 +31,10 @@ public class Finalizers { expect(! finalized); - System.gc(); + synchronized (lock) { + System.gc(); + lock.wait(5000); + } expect(finalized); }