[PATCH 2/2] Cygwin: fork: Remember child not before success.

Michael Haubenwallner michael.haubenwallner@ssi-schaefer.com
Tue Apr 30 07:09:00 GMT 2019


Do not remember the child before it was successfully initialized, or we
would need more sophisticated cleanup on child initialization failure,
like cleaning up the process table and suppressing SIGCHILD delivery
with multiple threads ("waitproc") involved.
---
 winsup/cygwin/fork.cc | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 59b13806c..05d5bb915 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -181,7 +181,8 @@ frok::child (volatile char * volatile here)
   cygheap->fdtab.fixup_after_fork (hParent);
 
   /* Signal that we have successfully initialized, so the parent can
-     - transfer data/bss for dynamically loaded dlls (if any), or
+     - transfer data/bss for dynamically loaded dlls (if any), and
+     - start up some tracker threads to remember the child, or
      - terminate the current fork call even if the child is initialized. */
   sync_with_parent ("performed fork fixups and dynamic dll loading", true);
 
@@ -411,20 +412,6 @@ frok::parent (volatile char * volatile stack_here)
   child.hProcess = hchild;
   ch.postfork (child);
 
-  /* Hopefully, this will succeed.  The alternative to doing things this
-     way is to reserve space prior to calling CreateProcess and then fill
-     it in afterwards.  This requires more bookkeeping than I like, though,
-     so we'll just do it the easy way.  So, terminate any child process if
-     we can't actually record the pid in the internal table. */
-  if (!child.remember (false))
-    {
-      this_errno = EAGAIN;
-#ifdef DEBUGGING0
-      error ("child remember failed");
-#endif
-      goto cleanup;
-    }
-
   /* CHILD IS STOPPED */
   debug_printf ("child is alive (but stopped)");
 
@@ -508,12 +495,28 @@ frok::parent (volatile char * volatile stack_here)
 	}
     }
 
+  /* Hopefully, this will succeed.  The alternative to doing things this
+     way is to reserve space prior to calling CreateProcess and then fill
+     it in afterwards.  This requires more bookkeeping than I like, though,
+     so we'll just do it the easy way.  So, terminate any child process if
+     we can't actually record the pid in the internal table. */
+  if (!child.remember (false))
+    {
+      this_errno = EAGAIN;
+#ifdef DEBUGGING0
+      error ("child remember failed");
+#endif
+      goto cleanup;
+    }
+
   /* Finally start the child up. */
   resume_child (forker_finished);
 
   ForceCloseHandle (forker_finished);
   forker_finished = NULL;
 
+  yield (); /* For child.remember (), to perform async thread startup. */
+
   return child_pid;
 
 /* Common cleanup code for failure cases */
-- 
2.19.2



More information about the Cygwin-patches mailing list