From be6896b8a05596f382d0a95c51885a9e4b5ea9ab Mon Sep 17 00:00:00 2001
From: Joel Dice <joel.dice@gmail.com>
Date: Fri, 3 Feb 2012 17:20:20 -0700
Subject: [PATCH] avoid running out of OS resources due to zombie thread
 accumulation (part 2)

My previous attempt wasn't quite sufficient, since it was too late to
call join on a thread which had already exited given the code was
written to aggressively dispose of system handles as soon as the
thread exited.  The solution is to delay disposing these handles until
after we're able to join the thread.
---
 src/machine.cpp | 66 ++++++++++++++++---------------------------------
 src/machine.h   | 14 +++--------
 2 files changed, 25 insertions(+), 55 deletions(-)

diff --git a/src/machine.cpp b/src/machine.cpp
index c9026e5360..73de8e32d3 100644
--- a/src/machine.cpp
+++ b/src/machine.cpp
@@ -45,18 +45,10 @@ void
 join(Thread* t, Thread* o)
 {
   if (t != o) {
-    // todo: There's potentially a leak here on systems where we must
-    // call join on a thread in order to clean up all resources
-    // associated with it.  If a thread has already been zombified by
-    // the time we get here, acquireSystem will return false, which
-    // means we can't safely join it because the System::Thread may
-    // already have been disposed.  In that case, the thread has
-    // already exited (or will soon), but the OS will never free all
-    // its resources because it doesn't know we're completely done
-    // with it.
-    if (acquireSystem(t, o)) {
+    assert(t, o->state != Thread::JoinedState);
+    assert(t, (o->flags & Thread::SystemFlag) == 0);
+    if (o->flags & Thread::JoinFlag) {
       o->systemThread->join();
-      releaseSystem(t, o);
     }
     o->state = Thread::JoinedState;
   }
@@ -257,15 +249,17 @@ killZombies(Thread* t, Thread* o)
     killZombies(t, child);
   }
 
-  switch (o->state) {
-  case Thread::ZombieState:
-    join(t, o);
-    // fall through
-
-  case Thread::JoinedState:
-    dispose(t, o, true);
-
-  default: break;
+  if ((o->flags & Thread::SystemFlag) == 0) {
+    switch (o->state) {
+    case Thread::ZombieState:
+      join(t, o);
+      // fall through
+      
+    case Thread::JoinedState:
+      dispose(t, o, true);
+      
+    default: break;
+    }
   }
 }
 
@@ -2623,23 +2617,7 @@ Thread::exit()
     } else {
       threadPeer(this, javaThread) = 0;
 
-      System::Monitor* myLock = lock;
-      System::Thread* mySystemThread = systemThread;
-
-      { ACQUIRE_RAW(this, m->stateLock);
-
-        while (flags & SystemFlag) {
-          m->stateLock->wait(systemThread, 0);
-        }
-
-        atomicOr(&flags, Thread::DisposeFlag);
-      
-        enter(this, Thread::ZombieState);
-      }
-
-      myLock->dispose();
-
-      mySystemThread->dispose();
+      enter(this, Thread::ZombieState);
     }
   }
 }
@@ -2647,14 +2625,12 @@ Thread::exit()
 void
 Thread::dispose()
 {
-  if ((flags & Thread::DisposeFlag) == 0) {
-    if (lock) {
-      lock->dispose();
-    }
-
-    if (systemThread) {
-      systemThread->dispose();
-    }
+  if (lock) {
+    lock->dispose();
+  }
+  
+  if (systemThread) {
+    systemThread->dispose();
   }
 
   -- m->threadCount;
diff --git a/src/machine.h b/src/machine.h
index 0841eb4443..12aca3bc45 100644
--- a/src/machine.h
+++ b/src/machine.h
@@ -1392,7 +1392,7 @@ class Thread {
   static const unsigned StressFlag = 1 << 4;
   static const unsigned ActiveFlag = 1 << 5;
   static const unsigned SystemFlag = 1 << 6;
-  static const unsigned DisposeFlag = 1 << 7;
+  static const unsigned JoinFlag = 1 << 7;
 
   class Protector {
    public:
@@ -1988,6 +1988,7 @@ runThread(Thread* t, uintptr_t*)
 inline bool
 startThread(Thread* t, Thread* p)
 {
+  p->flags |= Thread::JoinFlag;
   return t->m->system->success(t->m->system->start(&(p->runnable)));
 }
 
@@ -2784,19 +2785,12 @@ 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)) {
+  if (t->state != Thread::JoinedState) {
     atomicOr(&(target->flags), Thread::SystemFlag);
     return true;
   } else {
@@ -2809,7 +2803,7 @@ releaseSystem(Thread* t, Thread* target)
 {
   ACQUIRE_RAW(t, t->m->stateLock);
 
-  assert(t, not zombified(target));
+  assert(t, t->state != Thread::JoinedState);
 
   atomicAnd(&(target->flags), ~Thread::SystemFlag);
 }