From f03ea8e1c57bd5cea83f6cd47fa02870bdfeb1c5 Mon Sep 17 00:00:00 2001 From: Michael Haubenwallner Date: Thu, 2 May 2019 12:12:44 +0200 Subject: [PATCH] Cygwin: fork: Remember child not before success. 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. Compared to that, the potential slowdown due to an extra yield () call should be negligible. --- winsup/cygwin/fork.cc | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc index 59b13806c..c69081fc0 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,6 +495,20 @@ 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);