fix race condition in monitorRelease

There was an unlikely but dangerous race condition in monitorRelease
such that when a thread released a monitor and then tried to notify
the next thread in line, the latter thread might exit before it can be
notified.  This potentially led to a crash as the former thread tried
to acquire and notify the latter thread's private lock after it had
been disposed.

The solution is to do as we do in the interrupt and join cases: call
acquireSystem first and thereby either block the target thread from
exiting until we're done or find that it has already exited, in which
case nothing needs to be done.

I also looked at monitorNotify to see if we have a similar bug there,
but in that case the target thread can't exit without first acquiring
and releasing the monitor, and since we ensure that no thread can
execute monitorNotify without holding the monitor, there's no
potential for a race.
This commit is contained in:
Joel Dice 2010-12-16 16:46:25 -07:00
parent 6c53068f4f
commit cac2d2cac5

View File

@ -2398,6 +2398,36 @@ parameterFootprint(Thread* t, const char* s, bool static_);
void
addFinalizer(Thread* t, object target, void (*finalize)(Thread*, object));
inline bool
zombified(Thread* t)
{
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 bool
atomicCompareAndSwapObject(Thread* t, object target, unsigned offset,
object old, object new_)
@ -2545,10 +2575,12 @@ monitorRelease(Thread* t, object monitor)
Thread* next = monitorAtomicPollAcquire(t, monitor, false);
if (next) {
if (next and acquireSystem(t, next)) {
ACQUIRE(t, next->lock);
next->lock->notify(t->systemThread);
releaseSystem(t, next);
}
}
}
@ -2833,36 +2865,6 @@ notifyAll(Thread* t, object o)
}
}
inline bool
zombified(Thread* t)
{
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)
{