From aea02e545f5dea8fdfacf3a61110291229a75ead Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Tue, 16 Nov 2010 10:50:19 -0700 Subject: [PATCH] fix race condition in interrupting/joining threads as they are exiting My recent commit to ensure that OS resources are released immediately upon thread exit introduced a race condition where interrupting or joining a thread as it exited could lead to attempts to use already-released resources. This commit adds locking to avoid the race. --- src/machine.cpp | 15 +++++++++++---- src/machine.h | 40 +++++++++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/machine.cpp b/src/machine.cpp index a3d9402941..0191e1d285 100644 --- a/src/machine.cpp +++ b/src/machine.cpp @@ -45,10 +45,9 @@ void join(Thread* t, Thread* o) { if (t != o) { - if (o->state != Thread::ZombieState - and o->state != Thread::JoinedState) - { + if (acquireSystem(t, o)) { o->systemThread->join(); + releaseSystem(t, o); } o->state = Thread::JoinedState; } @@ -2407,7 +2406,15 @@ Thread::exit() turnOffTheLights(this); } else { threadPeer(this, javaThread) = 0; - enter(this, Thread::ZombieState); + + { ACQUIRE_RAW(this, m->stateLock); + + while (flags & SystemFlag) { + m->stateLock->wait(systemThread, 0); + } + + enter(this, Thread::ZombieState); + } lock->dispose(); lock = 0; diff --git a/src/machine.h b/src/machine.h index 21849fc04d..77c821e522 100644 --- a/src/machine.h +++ b/src/machine.h @@ -1298,6 +1298,7 @@ class Thread { static const unsigned DaemonFlag = 1 << 3; static const unsigned StressFlag = 1 << 4; static const unsigned ActiveFlag = 1 << 5; + static const unsigned SystemFlag = 1 << 6; class Protector { public: @@ -2758,13 +2759,42 @@ notifyAll(Thread* t, object o) } } -inline void -interrupt(Thread*, Thread* target) +inline bool +zombified(Thread* t) { - if (target->state != Thread::ZombieState - and target->state != Thread::JoinedState) - { + return t->state == Thread::ZombieState + or t->state == Thread::JoinedState; +} + +inline bool +acquireSystem(Thread* t, Thread* target) +{ + ACQUIRE_RAW(t, t->m->stateLock); + + if (not zombified(target)) { + atomicOr(&(target->flags), Thread::SystemFlag); + return true; + } else { + return false; + } +} + +inline void +releaseSystem(Thread* t, Thread* target) +{ + ACQUIRE_RAW(t, t->m->stateLock); + + assert(t, not zombified(target)); + + atomicAnd(&(target->flags), ~Thread::SystemFlag); +} + +inline void +interrupt(Thread* t, Thread* target) +{ + if (acquireSystem(t, target)) { target->systemThread->interrupt(); + releaseSystem(t, target); } }