Changeset 3460 in kBuild

Timestamp:

Sep 15, 2020 12:31:01 PM (5 years ago)

Author:

bird

Message:

kash: Use event semaphores to signal parent shell early before the subshell thread has performed cleanups.

Location:

trunk/src/kash

Files:

• shfork-win.c (modified) (1 diff)
• shinstance.c (modified) (10 diffs)
• shinstance.h (modified) (4 diffs)

trunk/src/kash/shfork-win.c

@@ -246 +246 @@
                 if (ResumeThread(ProcInfo.hThread) != (DWORD)-1)
                 {
-                    rc = sh_add_child(psh, ProcInfo.dwProcessId, ProcInfo.hProcess, K_TRUE);
+                    rc = sh_add_child(psh, ProcInfo.dwProcessId, ProcInfo.hProcess, NULL);
                     if (!rc)
                         rc = (int)ProcInfo.dwProcessId;

trunk/src/kash/shinstance.c

@@ -73 +73 @@
 /** Mutex serializing exec/spawn to prevent unwanted file inherting. */
 shmtx               g_sh_exec_inherit_mtx;
+/** Mutex protecting g_sh_sts_free. */
+static shmtx        g_sh_sts_mtx;
+/** List of free subshell status structure (saves CreateEvent calls).  */
+static shsubshellstatus * volatile g_sh_sts_free = NULL;
 #endif
 /** The mutex protecting the the globals and some shell instance members (sigs). */
@@ -708 +712 @@
 #ifndef SH_FORKED_MODE
     shmtx_init(&g_sh_exec_inherit_mtx);
+    shmtx_init(&g_sh_sts_mtx);
 #endif
 
@@ -1415 +1420 @@
 }
 
+#ifndef SH_FORKED_MODE
+
+/**
+ * Retains a reference to a subshell status structure.
+ */
+static unsigned shsubshellstatus_retain(shsubshellstatus *sts)
+{
+    unsigned refs = sh_atomic_dec(&sts->refs);
+    assert(refs > 1);
+    assert(refs < 16);
+    return refs;
+}
+
+/**
+ * Releases a reference to a subshell status structure.
+ */
+static unsigned shsubshellstatus_release(shinstance *psh, shsubshellstatus *sts)
+{
+    unsigned refs = sh_atomic_dec(&sts->refs);
+    assert(refs < ~(unsigned)0/4);
+    if (refs == 0)
+    {
+        shmtxtmp tmp;
+        shmtx_enter(&g_sh_sts_mtx, &tmp);
+        sts->next = g_sh_sts_free;
+        g_sh_sts_free = sts;
+        shmtx_leave(&g_sh_sts_mtx, &tmp);
+    }
+    return refs;
+}
+
+/**
+ * Creates a subshell status structure.
+ */
+static shsubshellstatus *shsubshellstatus_create(shinstance *psh, int refs)
+{
+    shsubshellstatus *sts;
+
+    /* Check the free list: */
+    if (g_sh_sts_free)
+    {
+        shmtxtmp tmp;
+        shmtx_enter(&g_sh_sts_mtx, &tmp);
+        sts = g_sh_sts_free;
+        if (sts)
+            g_sh_sts_free = sts->next;
+        shmtx_leave(&g_sh_sts_mtx, &tmp);
+    }
+    else
+        sts = NULL;
+    if (sts)
+    {
+# if K_OS == K_OS_WINDOWS
+        BOOL rc = ResetEvent((HANDLE)sts->towaiton);
+        assert(rc); K_NOREF(rc);
+# endif
+    }
+    else
+    {
+        /* Create a new one: */
+        sts = (shsubshellstatus *)sh_malloc(psh, sizeof(*sts));
+        if (!sts)
+            return NULL;
+# if K_OS == K_OS_WINDOWS
+        sts->towaiton = (void *)CreateEventW(NULL /*noinherit*/, TRUE /*fManualReset*/,
+                                             FALSE /*fInitialState*/, NULL /*pszName*/);
+        if (!sts->towaiton)
+        {
+            assert(0);
+            sh_free(psh, sts);
+            return NULL;
+        }
+# endif
+    }
+
+    /* Initialize it: */
+    sts->refs     = refs;
+    sts->status   = 999999;
+    sts->done     = 0;
+    sts->next     = NULL;
+# if K_OS == K_OS_WINDOWS
+    sts->hThread  = 0;
+# endif
+    return sts;
+}
+
+#endif /* !SH_FORKED_MODE */
+
 /**
  * Adds a child to the shell
@@ -1422 +1515 @@
  * @param   psh         The shell instance.
  * @param   pid         The child pid.
- * @param   hChild      Windows child handle.
- * @param   fProcess    Set if process, clear if thread.
+ * @param   hChild      Windows child wait handle (process if sts is NULL).
+ * @param   sts         Subshell status structure, NULL if progress.
  */
-int sh_add_child(shinstance *psh, shpid pid, void *hChild, KBOOL fProcess)
+int sh_add_child(shinstance *psh, shpid pid, void *hChild, shsubshellstatus *sts)
 {
     /* get a free table entry. */
@@ -1447 +1540 @@
 #endif
 #ifndef SH_FORKED_MODE
-    psh->children[i].fProcess = fProcess;
-#endif
-    (void)hChild; (void)fProcess;
+    psh->children[i].subshellstatus = sts;
+#endif
+    (void)hChild; (void)sts;
     return 0;
 }
@@ -1499 +1592 @@
     shinstance * volatile volpsh = (shinstance *)user;
     shinstance *psh = (shinstance *)user;
+    shsubshellstatus *sts;
     struct jmploc exitjmp;
     int iExit;
@@ -1527 +1621 @@
         if (iExit == SH_EXIT_ZERO)
             iExit = 0;
+    }
+
+    /* Signal parent. */
+    sts = psh->subshellstatus;
+    if (sts)
+    {
+        BOOL rc;
+        HANDLE hThread;
+
+        sts->status = W_EXITCODE(iExit, 0);
+        sts->done   = K_TRUE;
+        rc = SetEvent((HANDLE)sts->towaiton); assert(rc); K_NOREF(rc);
+
+        hThread = (HANDLE)sts->hThread;
+        sts->hThread = 0;
+        rc = CloseHandle(hThread); assert(rc);
+
+        shsubshellstatus_release(psh, sts);
+        psh->subshellstatus = NULL;
     }
 
@@ -1546 +1659 @@
 {
 # ifdef _MSC_VER
-    unsigned tid = 0;
-    uintptr_t hThread;
     shpid pid;
 
-    pshchild->thread    = thread;
-    pshchild->threadarg = arg;
-    hThread = _beginthreadex(NULL /*security*/, 0 /*stack_size*/, sh_thread_wrapper, pshchild, 0 /*initflags*/, &tid);
-    if (hThread == -1)
-        return -errno;
-
-    pid = SHPID_MAKE(SHPID_GET_PID(pshparent->pid), tid);
-    pshchild->pid = pid;
-    pshchild->tid = tid;
-
-    if (sh_add_child(pshparent, pid, (void *)hThread, K_FALSE) != 0) {
-        return -ENOMEM;
-    }
+    shsubshellstatus *sts = shsubshellstatus_create(pshparent, 2);
+    pshchild->subshellstatus = sts;
+    if (sts)
+    {
+        unsigned tid = 0;
+        uintptr_t hThread;
+
+        pshchild->thread    = thread;
+        pshchild->threadarg = arg;
+
+        hThread = _beginthreadex(NULL /*security*/, 0 /*stack_size*/, sh_thread_wrapper, pshchild, 0 /*initflags*/, &tid);
+        sts->hThread = hThread;
+        if (hThread != -1)
+        {
+            pid = SHPID_MAKE(SHPID_GET_PID(pshparent->pid), tid);
+            pshchild->pid = pid;
+            pshchild->tid = tid;
+
+            if (sh_add_child(pshparent, pid, sts->towaiton, sts) == 0)
+            {
+                return pid;
+            }
+
+            shsubshellstatus_retain(sts);
+            pid = -ENOMEM;
+        }
+        else
+            pid = -errno;
+        shsubshellstatus_release(pshparent, sts);
+        shsubshellstatus_release(pshparent, sts);
+    }
+    else
+        pid = -ENOMEM;
     return pid;
 
@@ -1645 +1776 @@
     if (i < psh->num_children)
     {
+        BOOL rc;
         if (hChild != INVALID_HANDLE_VALUE)
         {
             DWORD dwExitCode = 127;
 #ifndef SH_FORKED_MODE
-            if (psh->children[i].fProcess ? GetExitCodeProcess(hChild, &dwExitCode) : GetExitCodeThread(hChild, &dwExitCode))
-#else
+            if (psh->children[i].subshellstatus)
+            {
+                rc = psh->children[i].subshellstatus->done;
+                assert(rc);
+                if (rc)
+                {
+                    *statusp = psh->children[i].subshellstatus->status;
+                    pidret = psh->children[i].pid;
+                }
+            }
+            else
+#endif
             if (GetExitCodeProcess(hChild, &dwExitCode))
-#endif
             {
                 pidret = psh->children[i].pid;
@@ -1663 +1804 @@
         }
 
-        /* remove and close */
-        hChild = psh->children[i].hChild;
+        /* close and remove */
+        if (psh->children[i].subshellstatus)
+        {
+            shsubshellstatus_release(psh, psh->children[i].subshellstatus);
+            psh->children[i].subshellstatus = NULL;
+        }
+        else
+        {
+            rc = CloseHandle(psh->children[i].hChild);
+            assert(rc);
+        }
+
         psh->num_children--;
         if (i < psh->num_children)
             psh->children[i] = psh->children[psh->num_children];
-        i = CloseHandle(hChild); assert(i);
+        psh->children[psh->num_children].hChild = NULL;
+        psh->children[psh->num_children].subshellstatus = NULL;
     }
 

trunk/src/kash/shinstance.h

@@ -64 +64 @@
 #endif
 
+#ifndef SH_FORKED_MODE
+/**
+ * Subshell status.
+ */
+typedef struct shsubshellstatus
+{
+    unsigned volatile   refs;           /**< Reference counter. */
+    int volatile        status;         /**< The exit code. */
+    KBOOL volatile      done;           /**< Set if done (valid exit code). */
+    void               *towaiton;       /**< Event semaphore / whatever to wait on. */
+# if K_OS == K_OS_WINDOWS
+    uintptr_t volatile  hThread;        /**< The thread handle (child closes this). */
+# endif
+    struct shsubshellstatus *next;      /**< Next free one on the free chain. */
+} shsubshellstatus;
+#endif
+
 /**
  * A child process.
@@ -69 +86 @@
 typedef struct shchild
 {
-    shpid       pid;                    /**< The pid. */
+    shpid               pid;            /**< The pid. */
 #if K_OS == K_OS_WINDOWS
-    void       *hChild;                 /**< The process handle. */
+    void               *hChild;         /**< The handle to wait on. */
 #endif
 #ifndef SH_FORKED_MODE
-    KBOOL       fProcess;               /**< Set if process, clear if internal thread. */
+    shsubshellstatus   *subshellstatus; /**< Pointer to the subshell status structure.  NULL if child process. */
 #endif
 } shchild;
@@ -215 +232 @@
     void               *threadarg;      /**< The thread argument. */
     struct jmploc      *exitjmp;        /**< Long jump target in sh_thread_wrapper for use by sh__exit. */
+    shsubshellstatus   *subshellstatus; /**< Pointer to the subshell status structure (NULL if root). */
 #endif
 
@@ -496 +514 @@
 
 /* wait / process */
-int sh_add_child(shinstance *psh, shpid pid, void *hChild, KBOOL fProcess);
+int sh_add_child(shinstance *psh, shpid pid, void *hChild, shsubshellstatus *sts);
 #ifdef _MSC_VER
 #   include <process.h>