From cac2d2cac549d73b3476505fd5bbcb6a74812a30 Mon Sep 17 00:00:00 2001
From: Joel Dice <joel.dice@gmail.com>
Date: Thu, 16 Dec 2010 16:46:25 -0700
Subject: [PATCH] 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.
---
 src/machine.h | 64 ++++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/src/machine.h b/src/machine.h
index fbb5771c8c..3333cb084a 100644
--- a/src/machine.h
+++ b/src/machine.h
@@ -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)
 {