From 0aa5755187f294b55c81965289f39128acc8d5b5 Mon Sep 17 00:00:00 2001
From: Joel Dice <joel.dice@gmail.com>
Date: Thu, 12 Jan 2012 11:00:58 -0700
Subject: [PATCH] call C library free directly instead of System::free where
 possible

There was a subtle race condition in the VM shutdown process such that
a System::Thread would be disposed after the System instance it was
created under has been disposed, in which case doing a virtual call to
System::free with that instance would potentially cause a crash.  The
solution is to just call the C library version of free directly, since
that's all System::free does.
---
 src/machine.cpp |  9 +++++++++
 src/posix.cpp   | 16 ++++++++--------
 src/windows.cpp | 14 +++++++-------
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/src/machine.cpp b/src/machine.cpp
index 5084bbcf53..b4db6ac1fa 100644
--- a/src/machine.cpp
+++ b/src/machine.cpp
@@ -45,6 +45,15 @@ 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)) {
       o->systemThread->join();
       releaseSystem(t, o);
diff --git a/src/posix.cpp b/src/posix.cpp
index f695b0b21d..074c6b379c 100644
--- a/src/posix.cpp
+++ b/src/posix.cpp
@@ -169,7 +169,7 @@ class MySystem: public System {
     }
 
     virtual void dispose() {
-      s->free(this);
+      ::free(this);
     }
 
     pthread_t thread;
@@ -197,7 +197,7 @@ class MySystem: public System {
 
     virtual void dispose() {
       pthread_mutex_destroy(&mutex);
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -410,7 +410,7 @@ class MySystem: public System {
     virtual void dispose() {
       expect(s, owner_ == 0);
       pthread_mutex_destroy(&mutex);
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -441,7 +441,7 @@ class MySystem: public System {
       int r UNUSED = pthread_key_delete(key);
       expect(s, r == 0);
 
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -468,7 +468,7 @@ class MySystem: public System {
       if (start_) {
         munmap(start_, length_);
       }
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -494,7 +494,7 @@ class MySystem: public System {
       if (directory) {
         closedir(directory);
       }
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -541,10 +541,10 @@ class MySystem: public System {
       }
 
       if (name_) {
-        s->free(name_);
+        ::free(const_cast<char*>(name_));
       }
 
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
diff --git a/src/windows.cpp b/src/windows.cpp
index 8f90f6618b..42a79d3024 100644
--- a/src/windows.cpp
+++ b/src/windows.cpp
@@ -119,7 +119,7 @@ class MySystem: public System {
       CloseHandle(event);
       CloseHandle(mutex);
       CloseHandle(thread);
-      s->free(this);
+      ::free(this);
     }
 
     HANDLE thread;
@@ -150,7 +150,7 @@ class MySystem: public System {
 
     virtual void dispose() {
       CloseHandle(mutex);
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -377,7 +377,7 @@ class MySystem: public System {
     virtual void dispose() {
       assert(s, owner_ == 0);
       CloseHandle(mutex);
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -408,7 +408,7 @@ class MySystem: public System {
       bool r UNUSED = TlsFree(key);
       assert(s, r);
 
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -472,7 +472,7 @@ class MySystem: public System {
       if (handle and handle != INVALID_HANDLE_VALUE) {
         FindClose(handle);
       }
-      s->free(this);
+      ::free(this);
     }
 
     System* s;
@@ -523,10 +523,10 @@ class MySystem: public System {
       }
 
       if (name_) {
-        s->free(name_);
+        ::free(const_cast<char*>(name_));
       }
 
-      s->free(this);
+      ::free(this);
     }
 
     System* s;