Changeset 3447 for trunk/src/kash/shinstance.c

Timestamp:

Sep 11, 2020, 3:22:14 PM (5 years ago)

Author:

bird

Message:

kash: Avoid handle inheritance on windows when possible, allowing concurrent CreateProcess calls.

File:

• trunk/src/kash/shinstance.c (modified) (10 diffs)

trunk/src/kash/shinstance.c

@@ -50 +50 @@
 #if K_OS == K_OS_WINDOWS
 # include <Windows.h>
+# include "nt/nt_child_inject_standard_handles.h"
 # ifdef SH_FORKED_MODE
 extern pid_t shfork_do(shinstance *psh); /* shforkA-win.asm */
@@ -70 +71 @@
 #ifndef SH_FORKED_MODE
 /** Mutex serializing exec/spawn to prevent unwanted file inherting. */
-static shmtx        g_sh_exec_mtx;
+shmtx               g_sh_exec_inherit_mtx;
 #endif
 /** The mutex protecting the the globals and some shell instance members (sigs). */
@@ -82 +83 @@
 /** The number of shells. */
 static int volatile g_num_shells;
+/* Statistics: Number of subshells spawned. */
+static KU64             g_stat_subshells = 0;
+/* Statistics: Number of program exec'ed. */
+static KU64 volatile    g_stat_execs = 0;
+#if K_OS == K_OS_WINDOWS
+/* Statistics: Number of serialized exec calls. */
+static KU64 volatile    g_stat_execs_serialized = 0;
+#endif
 /** Per signal state for determining a common denominator.
  * @remarks defaults and unmasked actions aren't counted. */
@@ -172 +181 @@
     if (psh->rootshell)
         g_sh_root = psh;
+    else
+        g_stat_subshells++;
 
     psh->next = NULL;
@@ -637 +648 @@
     shmtx_init(&g_sh_mtx);
 #ifndef SH_FORKED_MODE
-    shmtx_init(&g_sh_exec_mtx);
+    shmtx_init(&g_sh_exec_inherit_mtx);
 #endif
 
@@ -1660 +1671 @@
     int rc;
 
+    g_stat_execs++;
+
 #ifdef DEBUG
     /* log it all */
@@ -1698 +1711 @@
          * This ain't quite straight forward on Windows...
          */
-#ifndef SH_FORKED_MODE
-        shmtxtmp tmp;
-#endif
         PROCESS_INFORMATION ProcInfo;
         STARTUPINFO StrtInfo;
-        intptr_t hndls[3];
+        shfdexecwin fdinfo;
         char *cwd = shfile_getcwd(&psh->fdtab, NULL, 0);
         char *cmdline;
@@ -1812 +1822 @@
 
         /* File handles. */
-#ifndef SH_FORKED_MODE
-        shmtx_enter(&g_sh_exec_mtx, &tmp);
-#endif
-        StrtInfo.dwFlags   |= STARTF_USESTDHANDLES;
-        StrtInfo.lpReserved2 = shfile_exec_win(&psh->fdtab, 1 /* prepare */, &StrtInfo.cbReserved2, hndls);
-        StrtInfo.hStdInput  = (HANDLE)hndls[0];
-        StrtInfo.hStdOutput = (HANDLE)hndls[1];
-        StrtInfo.hStdError  = (HANDLE)hndls[2];
+        fdinfo.strtinfo = &StrtInfo;
+        shfile_exec_win(&psh->fdtab, 1 /* prepare */, &fdinfo);
+        TRACE2((psh, "sh_execve: inherithandles=%d replacehandles={%d,%d,%d} handles={%p,%p,%p} suspended=%d Reserved2=%p LB %#x\n",
+                fdinfo.inherithandles, fdinfo.replacehandles[0], fdinfo.replacehandles[1], fdinfo.replacehandles[2],
+                fdinfo.handles[0], fdinfo.handles[1], fdinfo.handles[3], fdinfo.startsuspended,
+                StrtInfo.lpReserved2, StrtInfo.cbReserved2));
+        if (!fdinfo.inherithandles)
+        {
+            StrtInfo.dwFlags |= STARTF_USESTDHANDLES;
+            StrtInfo.hStdInput  = INVALID_HANDLE_VALUE;
+            StrtInfo.hStdOutput = INVALID_HANDLE_VALUE;
+            StrtInfo.hStdError  = INVALID_HANDLE_VALUE;
+        }
+        else
+        {
+            StrtInfo.dwFlags |= STARTF_USESTDHANDLES;
+            StrtInfo.hStdInput  = (HANDLE)fdinfo.handles[0];
+            StrtInfo.hStdOutput = (HANDLE)fdinfo.handles[1];
+            StrtInfo.hStdError  = (HANDLE)fdinfo.handles[2];
+            g_stat_execs_serialized++;
+        }
 
         /* Get going... */
-        rc = CreateProcess(exe,
-                           cmdline,
-                           NULL,         /* pProcessAttributes */
-                           NULL,         /* pThreadAttributes */
-                           TRUE,         /* bInheritHandles */
-                           0,            /* dwCreationFlags */
-                           envblock,
-                           cwd,
-                           &StrtInfo,
-                           &ProcInfo);
-
-        shfile_exec_win(&psh->fdtab, rc ? 0 /* done */ : -1 /* done but failed */, NULL, NULL);
-#ifndef SH_FORKED_MODE
-        shmtx_leave(&g_sh_exec_mtx, &tmp);
-#endif
+        rc = CreateProcessA(exe,
+                            cmdline,
+                            NULL,         /* pProcessAttributes */
+                            NULL,         /* pThreadAttributes */
+                            fdinfo.inherithandles,
+                            fdinfo.startsuspended ? CREATE_SUSPENDED : 0,
+                            envblock,
+                            cwd,
+                            &StrtInfo,
+                            &ProcInfo);
         if (rc)
         {
@@ -1842 +1860 @@
             DWORD dwExitCode;
 
+            if (fdinfo.startsuspended)
+            {
+                char errmsg[512];
+                rc = nt_child_inject_standard_handles(ProcInfo.hProcess, fdinfo.replacehandles, (HANDLE*)&fdinfo.handles[0],
+                                                      errmsg, sizeof(errmsg));
+                if (!rc)
+                {
+                    rc = ResumeThread(ProcInfo.hThread);
+                    if (!rc)
+                        TRACE2((psh, "sh_execve: ResumeThread failed: %u -> errno=ENXIO\n", GetLastError()));
+                }
+                else
+                {
+                    TRACE2((psh, "sh_execve: nt_child_inject_standard_handles failed: %d -> errno=ENXIO; %s\n", rc, errmsg));
+                    rc = FALSE;
+                }
+                errno = ENXIO;
+            }
+
+            shfile_exec_win(&psh->fdtab, rc ? 0 /* done */ : -1 /* done but failed */, &fdinfo);
+
             CloseHandle(ProcInfo.hThread);
-            dwErr = WaitForSingleObject(ProcInfo.hProcess, INFINITE);
-            assert(dwErr == WAIT_OBJECT_0);
-
-            if (GetExitCodeProcess(ProcInfo.hProcess, &dwExitCode))
+            ProcInfo.hThread = INVALID_HANDLE_VALUE;
+            if (rc)
             {
-                CloseHandle(ProcInfo.hProcess);
-                sh__exit(psh, dwExitCode);
+                /*
+                 * Wait for it and forward the exit code.
+                 */
+                dwErr = WaitForSingleObject(ProcInfo.hProcess, INFINITE);
+                assert(dwErr == WAIT_OBJECT_0);
+
+                if (GetExitCodeProcess(ProcInfo.hProcess, &dwExitCode))
+                {
+#ifndef SH_FORKED_MODE
+                    /** @todo signal the end of this subshell now, we can do the cleaning up
+                     *        after the parent shell has resumed. */
+#endif
+                    CloseHandle(ProcInfo.hProcess);
+                    ProcInfo.hProcess = INVALID_HANDLE_VALUE;
+                    sh__exit(psh, dwExitCode);
+                }
+
+                /* this shouldn't happen... */
+                TRACE2((psh, "sh_execve: GetExitCodeProcess failed: %u\n", GetLastError()));
+                assert(0);
+                errno = EINVAL;
             }
-            TRACE2((psh, "sh_execve: GetExitCodeProcess failed: %u\n", GetLastError()));
-            assert(0);
+            TerminateProcess(ProcInfo.hProcess, 0x40000015);
             CloseHandle(ProcInfo.hProcess);
-            errno = EINVAL;
         }
         else
         {
             DWORD dwErr = GetLastError();
+
+            shfile_exec_win(&psh->fdtab, -1 /* done but failed */, &fdinfo);
+
             switch (dwErr)
             {
@@ -1865 +1922 @@
                 case ERROR_BAD_EXE_FORMAT:          errno = ENOEXEC; break;
                 case ERROR_INVALID_EXE_SIGNATURE:   errno = ENOEXEC; break;
-                default:
-                    errno = EINVAL;
-                    break;
+                default:                            errno = EINVAL; break;
             }
             TRACE2((psh, "sh_execve: dwErr=%d -> errno=%d\n", dwErr, errno));
         }
-
     }
     rc = -1;