mjit_worker.c: suppress child process's output properly

Prior to this commit, some of parent process's output was unintentionally
suppressed. We couldn't suppress only child process's output with spawnvp.

Instead of that, this commit uses CreateProcess directly to redirect stdout
and stderr only for child process.

As it's dealing with HANDLE returned from CreateProcess, now waitpid macro
needs to CloseHandle it.

win32/win32.c: Introduce rb_w32_start_process which is designed for MJIT
worker. Other similar functions can't be used since they are using ALLOCV
that may trigger GC, which should be avoided on MJIT worker.


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@65033 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

2 files changed, 84 insertions, 51 deletions

diff --git a/mjit_worker.c b/mjit_worker.c
index e8f36b029c..a230ae823d 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -110,7 +110,7 @@
 #define dlclose(handle) (!FreeLibrary(handle))
 #define RTLD_NOW  -1
 
-#define waitpid(pid,stat_loc,options) (WaitForSingleObject((HANDLE)(pid), INFINITE), GetExitCodeProcess((HANDLE)(pid), (LPDWORD)(stat_loc)), (pid))
+#define waitpid(pid,stat_loc,options) (WaitForSingleObject((HANDLE)(pid), INFINITE), GetExitCodeProcess((HANDLE)(pid), (LPDWORD)(stat_loc)), CloseHandle((HANDLE)pid), (pid))
 #define WIFEXITED(S) ((S) != STILL_ACTIVE)
 #define WEXITSTATUS(S) (S)
 #define WIFSIGNALED(S) (0)
@@ -572,6 +572,17 @@ static pid_t
 start_process(const char *path, char *const *argv)
 {
     pid_t pid;
+    /*
+     * Not calling non-async-signal-safe functions between vfork
+     * and execv for safety
+     */
+    int dev_null = rb_cloexec_open(ruby_null_device, O_WRONLY, 0);
+    char fbuf[MAXPATHLEN];
+    const char *abspath = dln_find_exe_r(path, 0, fbuf, sizeof(fbuf));
+    if (!abspath) {
+        verbose(1, "MJIT: failed to find `%s' in PATH", path);
+        return -1;
+    }
 
     if (mjit_opts.verbose >= 2) {
         int i;
@@ -583,43 +594,42 @@ start_process(const char *path, char *const *argv)
         fprintf(stderr, "\n");
     }
 #ifdef _WIN32
-    pid = spawnvp(_P_NOWAIT, path, argv);
-#else
     {
-        /*
-         * Not calling non-async-signal-safe functions between vfork
-         * and execv for safety
-         */
-        char fbuf[MAXPATHLEN];
-        const char *abspath = dln_find_exe_r(path, 0, fbuf, sizeof(fbuf));
-        int dev_null;
-
-        if (!abspath) {
-            verbose(1, "MJIT: failed to find `%s' in PATH\n", path);
+        extern HANDLE rb_w32_start_process(const char *abspath, char *const *argv, int out_fd);
+        int out_fd = 0;
+        if (mjit_opts.verbose <= 1) {
+            /* Discard cl.exe's outputs like:
+                 _ruby_mjit_p12u3.c
+                   Creating library C:.../_ruby_mjit_p12u3.lib and object C:.../_ruby_mjit_p12u3.exp */
+            out_fd = dev_null;
+        }
+
+        pid = (pid_t)rb_w32_start_process(abspath, argv, out_fd);
+        if (pid == 0) {
+            verbose(1, "MJIT: Failed to create process: %s", dlerror());
             return -1;
         }
-        dev_null = rb_cloexec_open(ruby_null_device, O_WRONLY, 0);
-
-        if ((pid = vfork()) == 0) {
-            umask(0077);
-            if (mjit_opts.verbose == 0) {
-                /* CC can be started in a thread using a file which has been
-                   already removed while MJIT is finishing.  Discard the
-                   messages about missing files.  */
-                dup2(dev_null, STDERR_FILENO);
-                dup2(dev_null, STDOUT_FILENO);
-            }
-            (void)close(dev_null);
-            pid = execv(abspath, argv); /* Pid will be negative on an error */
-            /* Even if we successfully found CC to compile PCH we still can
-             fail with loading the CC in very rare cases for some reasons.
-             Stop the forked process in this case.  */
-            verbose(1, "MJIT: Error in execv: %s\n", abspath);
-            _exit(1);
+    }
+#else
+    if ((pid = vfork()) == 0) {
+        umask(0077);
+        if (mjit_opts.verbose == 0) {
+            /* CC can be started in a thread using a file which has been
+               already removed while MJIT is finishing.  Discard the
+               messages about missing files.  */
+            dup2(dev_null, STDERR_FILENO);
+            dup2(dev_null, STDOUT_FILENO);
         }
         (void)close(dev_null);
+        pid = execv(abspath, argv); /* Pid will be negative on an error */
+        /* Even if we successfully found CC to compile PCH we still can
+         fail with loading the CC in very rare cases for some reasons.
+         Stop the forked process in this case.  */
+        verbose(1, "MJIT: Error in execv: %s", abspath);
+        _exit(1);
     }
 #endif
+    (void)close(dev_null);
     return pid;
 }
 COMPILER_WARNING_POP
@@ -740,26 +750,7 @@ compile_c_to_so(const char *c_file, const char *so_file)
     if (args == NULL)
         return FALSE;
 
-    {
-        /* Discard cl.exe's outputs like:
-             _ruby_mjit_p12u3.c
-               Creating library C:.../_ruby_mjit_p12u3.lib and object C:.../_ruby_mjit_p12u3.exp */
-        int stdout_fileno, orig_fd, dev_null;
-        if (mjit_opts.verbose <= 1) {
-            stdout_fileno = _fileno(stdout);
-            orig_fd = dup(stdout_fileno);
-            dev_null = rb_cloexec_open(ruby_null_device, O_WRONLY, 0);
-
-            dup2(dev_null, stdout_fileno);
-        }
-        exit_code = exec_process(cc_path, args);
-        if (mjit_opts.verbose <= 1) {
-            dup2(orig_fd, stdout_fileno);
-
-            close(orig_fd);
-            close(dev_null);
-        }
-    }
+    exit_code = exec_process(cc_path, args);
     free(args);
 
     if (exit_code == 0) {
diff --git a/win32/win32.c b/win32/win32.c
index 8165812b25..9e319b3d1d 100644
--- a/win32/win32.c
+++ b/win32/win32.c
@@ -1298,6 +1298,47 @@ is_batch(const char *cmd)
 #define utf8_to_wstr(str, plen) mbstr_to_wstr(CP_UTF8, str, -1, plen)
 #define wstr_to_utf8(str, plen) wstr_to_mbstr(CP_UTF8, str, -1, plen)
 
+/* License: Ruby's */
+MJIT_FUNC_EXPORTED HANDLE
+rb_w32_start_process(const char *abspath, char *const *argv, int out_fd)
+{
+    /* NOTE: This function is used by MJIT worker, i.e. it must not touch Ruby VM.
+       This function is not using ALLOCV that may trigger GC. So this can't be
+       replaced with some families in this file. */
+    struct ChildRecord *child;
+    char *cmd;
+    size_t len;
+    WCHAR *wcmd = NULL, *wprog = NULL;
+    HANDLE outHandle = NULL;
+
+    if (out_fd) {
+        outHandle = (HANDLE)rb_w32_get_osfhandle(out_fd);
+    }
+
+    len = join_argv(NULL, argv, FALSE, filecp(), 1);
+    cmd = alloca(sizeof(char) * len);
+    join_argv(cmd, argv, FALSE, filecp(), 1);
+
+    if (!(wcmd = mbstr_to_wstr(filecp(), cmd, -1, NULL))) {
+        errno = E2BIG;
+        return 0;
+    }
+    if (!(wprog = mbstr_to_wstr(filecp(), abspath, -1, NULL))) {
+        errno = E2BIG;
+        return 0;
+    }
+
+    child = CreateChild(wcmd, wprog, NULL, NULL, outHandle, outHandle, 0);
+    if (child == NULL) {
+        return 0;
+    }
+    child->pid = 0; /* release the slot */
+
+    free(wcmd);
+    free(wprog);
+    return child->hProcess;
+}
+
 /* License: Artistic or GPL */
 static rb_pid_t
 w32_spawn(int mode, const char *cmd, const char *prog, UINT cp)
@@ -2112,6 +2153,7 @@ rb_w32_wstr_to_mbstr(UINT cp, const WCHAR *wstr, int clen, long *plen)
 WCHAR *
 rb_w32_mbstr_to_wstr(UINT cp, const char *str, int clen, long *plen)
 {
+    /* This is used by MJIT worker. Do not trigger GC or call Ruby method here. */
     WCHAR *ptr;
     int len = MultiByteToWideChar(cp, 0, str, clen, NULL, 0);
     if (!(ptr = malloc(sizeof(WCHAR) * len))) return 0;