New implementation approach for spawn_sync

include/uv.h

@@ -901,6 +901,12 @@ typedef struct uv_spawn_sync_t{
   int exit_timeout;
   int exit_code;
   int exit_signal;
+
+  /* private */
+  uv_pipe_t stdin_pipe;
+  uv_pipe_t stdout_pipe;
+  uv_pipe_t stderr_pipe;
+
 } uv_spawn_sync_t;
 
 int uv_spawn_sync(uv_loop_t* loop, uv_spawn_sync_t* spawn_sync);
@@ -1168,6 +1174,8 @@ struct uv_loop_s {
   void* data;
 };
 
+void uv_loop_init(uv_loop_t* loop);
+
 
 /* Don't export the private CPP symbols. */
 #undef UV_REQ_TYPE_PRIVATE

src/win/core.c

@@ -54,7 +54,7 @@ static void uv_init(void) {
 }
 
 
-static void uv_loop_init(uv_loop_t* loop) {
+void uv_loop_init(uv_loop_t* loop) {
   /* Create an I/O completion port */
   loop->iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 1);
   if (loop->iocp == NULL) {

src/win/process.c

@@ -814,7 +814,7 @@ static int uv_create_stdio_pipe_pair(uv_loop_t* loop, uv_pipe_t* server_pipe,
 static int duplicate_std_handle(uv_loop_t* loop, DWORD id, HANDLE* dup) {
   HANDLE handle;
   HANDLE current_process = GetCurrentProcess();
-  
+
   handle = GetStdHandle(id);
 
   if (handle == NULL) {
@@ -1043,3 +1043,107 @@ int uv_process_kill(uv_process_t* process, int signum) {
 
   return -1;
 }
+
+static void uv_spawn_sync_exit_cb(uv_process_t* process, int exit_code,
+    int term_signal) {
+
+  uv_spawn_sync_t* spawn_sync = (uv_spawn_sync_t*) process->data;
+  spawn_sync->exit_code = exit_code;
+  spawn_sync->exit_signal = term_signal;
+
+  /* Close the process handle */
+  uv_close((uv_handle_t*) process, NULL);
+
+  /* Close the pipes */
+  uv_close((uv_handle_t*) &spawn_sync->stdin_pipe, NULL);
+  uv_close((uv_handle_t*) &spawn_sync->stdout_pipe, NULL);
+  uv_close((uv_handle_t*) &spawn_sync->stderr_pipe, NULL);
+}
+
+
+static void spawn_sync_read_cb(uv_stream_t* handle, int nread, uv_buf_t buf) {
+  uv_spawn_sync_t* spawn_sync = (uv_spawn_sync_t*) handle->data;
+
+  if (nread < 0) {
+    if (uv_last_error(handle->loop).code == UV_EOF) {
+      // Pipe was closed
+      return;
+    } else {
+      // Read error. TODO: kill the process?
+      return;
+    }
+  }
+
+  if (handle == (uv_stream_t*) &spawn_sync->stdout_pipe) {
+    spawn_sync->stdout_read += nread;
+    if (spawn_sync->stdout_read == spawn_sync->stderr_size) {
+      uv_read_stop(handle);
+    }
+  } else if (handle == (uv_stream_t*) &spawn_sync->stderr_pipe) {
+    spawn_sync->stderr_read += nread;
+    if (spawn_sync->stderr_read == spawn_sync->stderr_size) {
+      uv_read_stop(handle);
+    }
+  } else {
+    abort();
+  }
+}
+
+
+static uv_buf_t spawn_sync_alloc_cb(uv_handle_t* handle, size_t suggested_size) {
+  uv_buf_t buf;
+  uv_spawn_sync_t* spawn_sync = (uv_spawn_sync_t*) handle->data;
+
+  /* Todo: handle overflows */
+
+  if (handle == (uv_handle_t*) &spawn_sync->stdout_pipe) {
+    buf.base = spawn_sync->stdout_buf + spawn_sync->stdout_read;
+    buf.len = spawn_sync->stdout_size - spawn_sync->stdout_read;
+  } else if (handle == (uv_handle_t*) &spawn_sync->stderr_pipe) {
+    buf.base = spawn_sync->stderr_buf + spawn_sync->stderr_read;
+    buf.len = spawn_sync->stderr_size - spawn_sync->stderr_read;
+  } else {
+    abort();
+  }
+
+  return buf;
+}
+
+
+int uv_spawn_sync(uv_loop_t* main_loop, uv_spawn_sync_t* spawn_sync) {
+  uv_loop_t subloop;
+  uv_process_t process;
+  uv_process_options_t process_options;
+
+  uv_loop_init(&subloop);
+
+  uv_pipe_init(&subloop, &spawn_sync->stdin_pipe);
+  uv_pipe_init(&subloop, &spawn_sync->stdout_pipe);
+  uv_pipe_init(&subloop, &spawn_sync->stderr_pipe);
+
+  memset((char*) &process_options, 0, sizeof process_options);
+  process_options.args = spawn_sync->args;
+  process_options.cwd = NULL;
+  process_options.env = NULL;
+  process_options.exit_cb = uv_spawn_sync_exit_cb;
+  process_options.file  = spawn_sync->file;
+  process_options.stdin_stream = &spawn_sync->stdin_pipe;
+  process_options.stdout_stream = &spawn_sync->stdout_pipe;
+  process_options.stderr_stream = &spawn_sync->stderr_pipe;
+
+  if (uv_spawn(&subloop, &process, process_options) != 0) {
+    // Handle error. Difficult, must actually clean up all the mess we made
+    // so far (e.g. the loop, stdio pipes etc). Prolly just uv_run() the loop
+    // until it dies?
+  }
+
+  uv_read_start((uv_stream_t*) &spawn_sync->stdout_pipe, spawn_sync_alloc_cb, spawn_sync_read_cb);
+  uv_read_start((uv_stream_t*) &spawn_sync->stderr_pipe, spawn_sync_alloc_cb, spawn_sync_read_cb);
+
+  process.data = spawn_sync;
+  spawn_sync->stdin_pipe.data = spawn_sync;
+  spawn_sync->stdout_pipe.data = spawn_sync;
+  spawn_sync->stderr_pipe.data = spawn_sync;
+
+  uv_run(&subloop);
+}

test/run-tests.c

@@ -22,7 +22,12 @@
 #include <stdio.h>
 #include <string.h>
 
-#include <unistd.h>
+#ifndef _MSC_VER
+# include <unistd.h>
+#else
+# include <io.h>
+#endif
+
 #include "runner.h"
 #include "task.h"
 
@@ -115,8 +120,26 @@ static int maybe_run_test(int argc, char **argv) {
   }
 
   if (strcmp(argv[1], "spawn_helper_stdout_stderr") == 0) {
-    fprintf(stdout, "stdout\n");
-    fprintf(stderr, "stderr\n");
+    const char stdout_text[] = "stdout\n";
+    const char stderr_text[] = "stderr\n";
+
+#ifndef _WIN32
+    fprintf(stdout, stdout_text);
+    fprintf(stderr, stderr_text);
+#else
+    /* windows rage: can't tell crt to not convert line endings on stdout */
+    DWORD written;
+    WriteFile(GetStdHandle(STD_OUTPUT_HANDLE),
+              (void*) stdout_text,
+              strlen(stdout_text),
+              &written,
+              NULL);
+    WriteFile(GetStdHandle(STD_ERROR_HANDLE),
+              (void*) stderr_text,
+              strlen(stderr_text),
+              &written,
+              NULL);
+#endif
     return 0;
   }
 

test/test-spawn-sync.c

@@ -51,6 +51,8 @@ static void init_process_options(char* test) {
 }
 
 void debug(int r) {
+  uv_err_t err;
+
   fprintf(stderr, "----------------------------------------\n");
   fprintf(stderr, "r: %i\n", r);
   fprintf(stderr, "spawn.pid: %i\n", spawn.pid);
@@ -74,15 +76,14 @@ void debug(int r) {
   fprintf(stderr, "spawn.exit_code: %i\n", spawn.exit_code);
   fprintf(stderr, "spawn.exit_signal: %i\n", spawn.exit_signal);
 
-  uv_err_t err = uv_last_error(uv_default_loop());
+  err = uv_last_error(uv_default_loop());
   fprintf(stderr, "uv_last_error_name: %s\n", uv_err_name(err));
   fprintf(stderr, "uv_strerror: %s\n", uv_strerror(err));
   fprintf(stderr, "----------------------------------------\n");
 }
 
 TEST_IMPL(spawn_sync_exit_code) {
   int r;
-  uv_init();
 
   init_process_options("spawn_helper_exit_code");
 
@@ -92,22 +93,25 @@ TEST_IMPL(spawn_sync_exit_code) {
   ASSERT(spawn.pid >= 0);
   ASSERT(r == 0);
   ASSERT(spawn.exit_code == 1);
+#ifndef _WIN32
   ASSERT(spawn.exit_signal == -1);
+#endif
 
   return 0;
 }
 
 TEST_IMPL(spawn_sync_exit_signal) {
   int r;
-  uv_init();
 
   init_process_options("spawn_helper_exit_signal");
 
   r = uv_spawn_sync(uv_default_loop(), &spawn);
   debug(r);
 
   ASSERT(r == 0);
+#ifndef _WIN32
   ASSERT(spawn.exit_signal == SIGKILL);
+#endif
   ASSERT(spawn.exit_code == -1);
 
   return 0;
@@ -117,7 +121,6 @@ TEST_IMPL(spawn_sync_stdio) {
   int r;
   char *expected_stdout = "stdout\n";
   char *expected_stderr = "stderr\n";
-  uv_init();
 
   init_process_options("spawn_helper_stdout_stderr");
 
@@ -136,7 +139,6 @@ TEST_IMPL(spawn_sync_stdio) {
 TEST_IMPL(spawn_sync_stdout) {
   int r;
   char *expected_stdout = "stdout\n";
-  uv_init();
 
   init_process_options("spawn_helper_stdout_stderr");
 
@@ -146,7 +148,7 @@ TEST_IMPL(spawn_sync_stdout) {
   debug(r);
 
   ASSERT(r == 0);
-  ASSERT(strcmp(spawn.stdout_buf, expected_stdout) == 0);
+  ASSERT(strncmp(spawn.stdout_buf, expected_stdout, strlen(expected_stdout)) == 0);
   ASSERT(spawn.stdout_read == strlen(expected_stdout));
   ASSERT(spawn.stderr_read == 0);
 
@@ -156,7 +158,6 @@ TEST_IMPL(spawn_sync_stdout) {
 TEST_IMPL(spawn_sync_stderr) {
   int r;
   char *expected_stderr = "stderr\n";
-  uv_init();
 
   init_process_options("spawn_helper_stdout_stderr");
 
@@ -175,7 +176,6 @@ TEST_IMPL(spawn_sync_stderr) {
 
 TEST_IMPL(spawn_sync_stdout_overflow) {
   int r;
-  uv_init();
 
   init_process_options("spawn_helper_stdout_stderr");
 
@@ -193,7 +193,6 @@ TEST_IMPL(spawn_sync_stdout_overflow) {
 
 TEST_IMPL(spawn_sync_stderr_overflow) {
   int r;
-  uv_init();
 
   init_process_options("spawn_helper_stdout_stderr");
 
@@ -212,7 +211,6 @@ TEST_IMPL(spawn_sync_stderr_overflow) {
 TEST_IMPL(spawn_sync_combine_stdio) {
   int r;
   char *expected_stdout = "stdout\nstderr\n";
-  uv_init();
 
   init_process_options("spawn_helper_stdout_stderr");
 
@@ -231,7 +229,6 @@ TEST_IMPL(spawn_sync_combine_stdio) {
 
 TEST_IMPL(spawn_sync_stdin) {
   int r;
-  uv_init();
 
   init_process_options("spawn_helper_stdin");
 
@@ -250,7 +247,6 @@ TEST_IMPL(spawn_sync_stdin) {
 
 TEST_IMPL(spawn_sync_stdin_stream) {
   int r;
-  uv_init();
 
   init_process_options("spawn_helper_stdin_stream");
 
@@ -271,7 +267,6 @@ TEST_IMPL(spawn_sync_stdin_stream) {
 
 TEST_IMPL(spawn_sync_timeout) {
   int r;
-  uv_init();
 
   init_process_options("spawn_helper_timeout");
 

uv.gyp

@@ -260,6 +260,7 @@
         'test/test-ref.c',
         'test/test-shutdown-eof.c',
         'test/test-spawn.c',
+        'test/test-spawn-sync.c',
         'test/test-tcp-bind-error.c',
         'test/test-tcp-bind6-error.c',
         'test/test-tcp-close.c',