Reuse thread when writing stdout/stderr on Windows.

runtime/bin/eventhandler_win.cc

Index: runtime/bin/eventhandler_win.cc
diff --git a/runtime/bin/eventhandler_win.cc b/runtime/bin/eventhandler_win.cc
index 59b22077d4d5a8f67257595bb9c41face06fa7a4..16e195823a859ae6d47c6f644b0e49956cf513b7 100644
--- a/runtime/bin/eventhandler_win.cc
+++ b/runtime/bin/eventhandler_win.cc
@@ -20,6 +20,7 @@
 #include "bin/socket.h"
 #include "bin/utils.h"
 #include "platform/thread.h"
+#include "vm/thread.h"
 
 
 namespace dart {
@@ -560,9 +561,27 @@ int Handle::Read(void* buffer, int num_bytes) {
 }
 
 

[Søren Gjesse, 2013/10/02 08:49:22]

Please add these static variables to the Windows EventHandlerImplementation
class.

[Anders Johnsen, 2013/10/02 10:16:59]

Done.

+static Monitor* write_monitor = new Monitor();
+static bool write_thread_exists = false;
+static bool write_thread_running = false;
+static Handle* write_handle = NULL;
+
 static unsigned int __stdcall WriteFileThread(void* args) {
-  Handle* handle = reinterpret_cast<Handle*>(args);
-  handle->WriteSyncCompleteAsync();
+  MonitorLocker locker(write_monitor);
+  write_thread_running = true;
+  // Notify we have started.
+  locker.Notify();
+
+  while (write_thread_running) {
+    locker.Wait(Monitor::kNoTimeout);
+    if (write_handle) {
+      // We woke up and had a handle. Complete it.
+      write_handle->WriteSyncCompleteAsync();
+      write_handle = NULL;
+    }
+  }
+  write_thread_exists = false;
+  locker.Notify();
   return 0;
 }
 
@@ -606,24 +625,43 @@ int Handle::Write(const void* buffer, int num_bytes) {
     return num_bytes;
   } else {
     // In the case of stdout and stderr, OverlappedIO is not supported.
-    // Here we'll instead spawn a new thread for each write, to make it async.
+    // Here we'll instead use a thread, to make it async.
     // This code is actually never exposed to the user, as stdout and stderr is
     // not available as a RawSocket, but only wrapped in a Socket.
     // Note that we return '0', unless a thread have already completed a write.
-    // TODO(ajohnsen): Don't spawn a new thread per write. Issue 13541.
     if (thread_wrote_ > 0) {
       if (num_bytes > thread_wrote_) num_bytes = thread_wrote_;
       thread_wrote_ -= num_bytes;
       return num_bytes;
     }
-    pending_write_ = OverlappedBuffer::AllocateWriteBuffer(num_bytes);
-    pending_write_->Write(buffer, num_bytes);
-    // Completing asynchronously through thread.
-    uint32_t tid;
-    uintptr_t thread_handle =
-        _beginthreadex(NULL, 32 * 1024, WriteFileThread, this, 0, &tid);
-    if (thread_handle == -1) {
-      FATAL("Failed to start write file thread");
+    // Double-checked locking, to avoid taking the lock in most cases.

[Søren Gjesse, 2013/10/02 08:49:22]

You need to hold the lock before accessing shared variables. There is no
guarantee that these reads are atomic (in almost all cases they are, but the
compiler might fool us).

[Anders Johnsen, 2013/10/02 10:16:59]

Done.

+    if (!write_thread_exists) {
+      MonitorLocker locker(write_monitor);
+      if (!write_thread_exists) {
+        write_thread_exists = true;
+        uint32_t tid;
+        uintptr_t thread_handle =
+            _beginthreadex(NULL, 32 * 1024, WriteFileThread, this, 0, &tid);
+        // Completing asynchronously through thread.
+        if (thread_handle == -1) {
+          FATAL("Failed to start write file thread");
+        }
+        while (!write_thread_running) {
+          // Wait until we the thread is running.
+          locker.Wait(Monitor::kNoTimeout);
+        }
+      }
+    }
+    // Double-checked locking, to avoid taking the lock in most cases.
+    if (write_handle == NULL) {
+      MonitorLocker locker(write_monitor);
+      if (write_handle == NULL) {
+        // Create buffer and notify thread about the new handle.
+        pending_write_ = OverlappedBuffer::AllocateWriteBuffer(num_bytes);
+        pending_write_->Write(buffer, num_bytes);
+        write_handle = this;
+        locker.Notify();
+      }
     }
     return 0;
   }
@@ -999,6 +1037,15 @@ EventHandlerImplementation::EventHandlerImplementation() {
 
 
 EventHandlerImplementation::~EventHandlerImplementation() {

[Søren Gjesse, 2013/10/02 08:49:22]

Take the lock before looking at shared data.

[Anders Johnsen, 2013/10/02 10:16:59]

Done.

+  if (write_thread_exists) {
+    ASSERT(write_handle == NULL);
+    MonitorLocker locker(write_monitor);
+    write_thread_running = false;
+    locker.Notify();
+    while (write_thread_exists) {
+      locker.Wait(Monitor::kNoTimeout);
+    }
+  }
   CloseHandle(completion_port_);
 }