Automatic thread cleanup on non-Windows platforms.

Issue 23474 is tracking Windows support (use Chromium's method).

BUG=23474
R=iposva@google.com

Review URL: https://codereview.chromium.org//1134003005

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@45817 260f80e4-7a28-3924-810f-c04153c831b5
diff --git a/dart/runtime/vm/dart.cc b/dart/runtime/vm/dart.cc
index 2dc72ad..d14ba97 100644
--- a/dart/runtime/vm/dart.cc
+++ b/dart/runtime/vm/dart.cc
@@ -117,6 +117,7 @@
     ASSERT(vm_isolate_ == NULL);
     ASSERT(Flags::Initialized());
     const bool is_vm_isolate = true;
+    Thread::EnsureInit();
     vm_isolate_ = Isolate::Init("vm-isolate", is_vm_isolate);
     StackZone zone(vm_isolate_);
     HandleScope handle_scope(vm_isolate_);
@@ -206,6 +207,7 @@
   thread_pool_ = NULL;
 
   // Set the VM isolate as current isolate.
+  Thread::EnsureInit();
   Thread::EnterIsolate(vm_isolate_);
 
   // There is a planned and known asymmetry here: We exit one scope for the VM
diff --git a/dart/runtime/vm/dart_api_impl.cc b/dart/runtime/vm/dart_api_impl.cc
index 6f91580..a6feef0 100644
--- a/dart/runtime/vm/dart_api_impl.cc
+++ b/dart/runtime/vm/dart_api_impl.cc
@@ -1290,6 +1290,7 @@
                                             char** error) {
   CHECK_NO_ISOLATE(Isolate::Current());
   char* isolate_name = BuildIsolateName(script_uri, main);
+  Thread::EnsureInit();
   Isolate* isolate = Dart::CreateIsolate(isolate_name);
   free(isolate_name);
   {
@@ -1365,6 +1366,7 @@
   if (iso->mutator_thread() != NULL) {
     FATAL("Multiple mutators within one isolate is not supported.");
   }
+  Thread::EnsureInit();
   Thread::EnterIsolate(iso);
 }
 
diff --git a/dart/runtime/vm/os_thread.h b/dart/runtime/vm/os_thread.h
index e55380c..2dfd3f5 100644
--- a/dart/runtime/vm/os_thread.h
+++ b/dart/runtime/vm/os_thread.h
@@ -30,13 +30,15 @@
   static ThreadId kInvalidThreadId;
 
   typedef void (*ThreadStartFunction) (uword parameter);
+  typedef void (*ThreadDestructor) (void* parameter);
 
   // Start a thread running the specified function. Returns 0 if the
   // thread started successfuly and a system specific error code if
   // the thread failed to start.
   static int Start(ThreadStartFunction function, uword parameters);
 
-  static ThreadLocalKey CreateThreadLocal();
+  // NOTE: Destructor currently ignored on Windows (issue 23474).
+  static ThreadLocalKey CreateThreadLocal(ThreadDestructor destructor = NULL);
   static void DeleteThreadLocal(ThreadLocalKey key);
   static uword GetThreadLocal(ThreadLocalKey key) {
     return ThreadInlineImpl::GetThreadLocal(key);
diff --git a/dart/runtime/vm/os_thread_android.cc b/dart/runtime/vm/os_thread_android.cc
index 57de1b0..443e800 100644
--- a/dart/runtime/vm/os_thread_android.cc
+++ b/dart/runtime/vm/os_thread_android.cc
@@ -116,9 +116,9 @@
     static_cast<pthread_key_t>(-1);
 ThreadId OSThread::kInvalidThreadId = static_cast<ThreadId>(0);
 
-ThreadLocalKey OSThread::CreateThreadLocal() {
+ThreadLocalKey OSThread::CreateThreadLocal(ThreadDestructor destructor) {
   pthread_key_t key = kUnsetThreadLocalKey;
-  int result = pthread_key_create(&key, NULL);
+  int result = pthread_key_create(&key, destructor);
   VALIDATE_PTHREAD_RESULT(result);
   ASSERT(key != kUnsetThreadLocalKey);
   return key;
diff --git a/dart/runtime/vm/os_thread_linux.cc b/dart/runtime/vm/os_thread_linux.cc
index b1b97fd..5a0fe4f 100644
--- a/dart/runtime/vm/os_thread_linux.cc
+++ b/dart/runtime/vm/os_thread_linux.cc
@@ -117,9 +117,9 @@
     static_cast<pthread_key_t>(-1);
 ThreadId OSThread::kInvalidThreadId = static_cast<ThreadId>(0);
 
-ThreadLocalKey OSThread::CreateThreadLocal() {
+ThreadLocalKey OSThread::CreateThreadLocal(ThreadDestructor destructor) {
   pthread_key_t key = kUnsetThreadLocalKey;
-  int result = pthread_key_create(&key, NULL);
+  int result = pthread_key_create(&key, destructor);
   VALIDATE_PTHREAD_RESULT(result);
   ASSERT(key != kUnsetThreadLocalKey);
   return key;
diff --git a/dart/runtime/vm/os_thread_macos.cc b/dart/runtime/vm/os_thread_macos.cc
index 2102887..2b870db 100644
--- a/dart/runtime/vm/os_thread_macos.cc
+++ b/dart/runtime/vm/os_thread_macos.cc
@@ -109,9 +109,9 @@
     static_cast<pthread_key_t>(-1);
 ThreadId OSThread::kInvalidThreadId = reinterpret_cast<ThreadId>(NULL);
 
-ThreadLocalKey OSThread::CreateThreadLocal() {
+ThreadLocalKey OSThread::CreateThreadLocal(ThreadDestructor destructor) {
   pthread_key_t key = kUnsetThreadLocalKey;
-  int result = pthread_key_create(&key, NULL);
+  int result = pthread_key_create(&key, destructor);
   VALIDATE_PTHREAD_RESULT(result);
   ASSERT(key != kUnsetThreadLocalKey);
   return key;
diff --git a/dart/runtime/vm/os_thread_win.cc b/dart/runtime/vm/os_thread_win.cc
index ff6ced0..cdcf36c 100644
--- a/dart/runtime/vm/os_thread_win.cc
+++ b/dart/runtime/vm/os_thread_win.cc
@@ -72,7 +72,7 @@
 ThreadLocalKey OSThread::kUnsetThreadLocalKey = TLS_OUT_OF_INDEXES;
 ThreadId OSThread::kInvalidThreadId = 0;
 
-ThreadLocalKey OSThread::CreateThreadLocal() {
+ThreadLocalKey OSThread::CreateThreadLocal(ThreadDestructor unused) {
   ThreadLocalKey key = TlsAlloc();
   if (key == kUnsetThreadLocalKey) {
     FATAL1("TlsAlloc failed %d", GetLastError());
diff --git a/dart/runtime/vm/thread.cc b/dart/runtime/vm/thread.cc
index 693dd9a..79919d4 100644
--- a/dart/runtime/vm/thread.cc
+++ b/dart/runtime/vm/thread.cc
@@ -18,9 +18,14 @@
 ThreadLocalKey Thread::thread_key_ = OSThread::kUnsetThreadLocalKey;
 
 
+static void DeleteThread(void* thread) {
+  delete reinterpret_cast<Thread*>(thread);
+}
+
+
 void Thread::InitOnce() {
   ASSERT(thread_key_ == OSThread::kUnsetThreadLocalKey);
-  thread_key_ = OSThread::CreateThreadLocal();
+  thread_key_ = OSThread::CreateThreadLocal(DeleteThread);
   ASSERT(thread_key_ != OSThread::kUnsetThreadLocalKey);
 }
 
@@ -37,6 +42,7 @@
 }
 
 
+#if defined(TARGET_OS_WINDOWS)
 void Thread::CleanUp() {
   Thread* current = Current();
   if (current != NULL) {
@@ -44,11 +50,12 @@
   }
   SetCurrent(NULL);
 }
+#endif
 
 
 void Thread::EnterIsolate(Isolate* isolate) {
-  EnsureInit();
   Thread* thread = Thread::Current();
+  ASSERT(thread != NULL);
   ASSERT(thread->isolate() == NULL);
   ASSERT(isolate->mutator_thread() == NULL);
   thread->isolate_ = isolate;
@@ -87,8 +94,8 @@
 
 
 void Thread::EnterIsolateAsHelper(Isolate* isolate) {
-  EnsureInit();
   Thread* thread = Thread::Current();
+  ASSERT(thread != NULL);
   ASSERT(thread->isolate() == NULL);
   thread->isolate_ = isolate;
   // Do not update isolate->mutator_thread, but perform sanity check:
diff --git a/dart/runtime/vm/thread.h b/dart/runtime/vm/thread.h
index 2e8bbed..056430c 100644
--- a/dart/runtime/vm/thread.h
+++ b/dart/runtime/vm/thread.h
@@ -16,13 +16,9 @@
 
 // A VM thread; may be executing Dart code or performing helper tasks like
 // garbage collection or compilation. The Thread structure associated with
-// a thread is allocated when the thread enters an isolate, and destroyed
-// upon an explicit call to CleanUp.
-//
-// NOTE: When the underlying OS thread is started by the embedder, i.e., not
-// part of a ThreadPool managed by the VM, we currently leak the Thread
-// structure after its last isolate exit. We could add an explicit cleanup API,
-// or, on platforms that support it, register a cleanup callback.
+// a thread is allocated by EnsureInit before entering an isolate, and destroyed
+// automatically when the underlying OS thread exits. NOTE: On Windows, CleanUp
+// must currently be called manually (issue 23474).
 class Thread {
  public:
   // The currently executing thread, or NULL if not yet initialized.
@@ -30,7 +26,10 @@
     return reinterpret_cast<Thread*>(OSThread::GetThreadLocal(thread_key_));
   }
 
-  // Makes the current thread enter 'isolate'. Also calls EnsureInit.
+  // Initializes the current thread as a VM thread, if not already done.
+  static void EnsureInit();
+
+  // Makes the current thread enter 'isolate'.
   static void EnterIsolate(Isolate* isolate);
   // Makes the current thread exit its isolate.
   static void ExitIsolate();
@@ -42,10 +41,10 @@
   static void EnterIsolateAsHelper(Isolate* isolate);
   static void ExitIsolateAsHelper();
 
-  // Initializes the current thread as a VM thread, if not already done.
-  static void EnsureInit();
-  // Clears the state of the current thread.
+#if defined(TARGET_OS_WINDOWS)
+  // Clears the state of the current thread and frees the allocation.
   static void CleanUp();
+#endif
 
   // Called at VM startup.
   static void InitOnce();
diff --git a/dart/runtime/vm/thread_pool.cc b/dart/runtime/vm/thread_pool.cc
index f81f99a..5b3a713 100644
--- a/dart/runtime/vm/thread_pool.cc
+++ b/dart/runtime/vm/thread_pool.cc
@@ -274,11 +274,8 @@
 
     // Release monitor while handling the task.
     monitor_.Exit();
-    Thread::EnsureInit();
     task->Run();
     ASSERT(Isolate::Current() == NULL);
-    // Prevent unintended sharing of state between tasks.
-    Thread::CleanUp();
     delete task;
     monitor_.Enter();
 
@@ -318,6 +315,7 @@
 
 // static
 void ThreadPool::Worker::Main(uword args) {
+  Thread::EnsureInit();
   Worker* worker = reinterpret_cast<Worker*>(args);
   worker->Loop();
 
@@ -333,6 +331,9 @@
     ml.Notify();
   }
   delete worker;
+#if defined(TARGET_OS_WINDOWS)
+  Thread::CleanUp();
+#endif
 }
 
 }  // namespace dart