* child_info.h (CURR_CHILD_INFO_MAGIC): Reset.

(child_info::rd_proc_pipe): Declare new field.
(child_info::wr_proc_pipe): Ditto.
(child_info::prefork): Declare new function, derived from previous pinfo
version.
* dcrt0.cc (child_info_fork::handle_fork): Close previous wr_proc_pipe when
appropriate and assign new one from passed-in child_info block.
(child_info_spawn::handle_spawn): Assign our wr_proc_pipe handle from passed-in
child_info block.
* fork.cc (child_info::prefork): Define new function.
(frok::child): Clear rd_proc_pipe and wr_proc_pipe so they will not be closed
by the child_info destructor.
(frok::parent): Use child_info prefork handling, outside of retry loop.  Set
rd_proc_pipe in child's pinfo after successful CreateProcess.  Eliminate
postfork call.
* globals.cc (my_wr_proc_pipe): Define/declare new variable.
* pinfo.cc (pinfo::pending_rd_proc_pipe): Delete.
(pinfo::pending_wr_proc_pipe): Ditto.
(pinfo::prefork): Ditto.
(pinfo::postfork): Ditto.
(pinfo::postexec): Ditto.
(pinfo::wait): Assume that rd_proc_pipe is set up correctly prior to call.
(_pinfo::alert_parent): Replace "wr_proc_pipe" with "my_wr_proc_pipe".
* pinfo.h (_pinfo::_wr_proc_pipe): Delete declaration.
(_pinfo::set_rd_proc_pipe): Define new function.
(pinfo::pending_rd_proc_pipe): Delete declaration.
(pinfo::pending_wr_proc_pipe): Ditto.
(pinfo::prefork): Ditto.
(pinfo::postfork): Ditto.
(pinfo::postexec): Ditto.
(pinfo::wr_proc_pipe): Ditto.
* sigproc.cc (child_info::child_info): Clear rd_proc_pipe and wr_proc_pipe.
(child_info::cleanup): Close rd_proc_pipe and wr_proc_pipe if necessary.
(child_info_fork::child_info_fork): Set forker_finished to NULL by default.
(child_info_spawn::child_info_spawn): Use my_wr_proc_pipe rather than
myself->wr_proc_pipe.
(child_info::sync): Ditto.
(child_info_spawn::cleanup): Call child_info::cleanup.
* spawn.cc (child_info_spawn::worker): Remove call to myself.prefork().  Set
wr_proc_pipe when execing or set up new rd_proc_pipe/wr_proc_pipe via
child_info::prefork when spawning.  Remove call to pinfo::postexec.  Set
rd_proc_pipe in child pinfo when spawning.  Use my_wr_proc_pipe rather than
myself->wr_proc_pipe.  Remove call to postfork.

winsup/cygwin/ChangeLog

@@ -1,3 +1,53 @@
+2012-03-20  Christopher Faylor  <me.cygwin2012@cgf.cx>
+
+	* child_info.h (CURR_CHILD_INFO_MAGIC): Reset.
+	(child_info::rd_proc_pipe): Declare new field.
+	(child_info::wr_proc_pipe): Ditto.
+	(child_info::prefork): Declare new function, derived from previous
+	pinfo version.
+	* dcrt0.cc (child_info_fork::handle_fork): Close previous wr_proc_pipe
+	when appropriate and assign new one from passed-in child_info block.
+	(child_info_spawn::handle_spawn): Assign our wr_proc_pipe handle from
+	passed-in child_info block.
+	* fork.cc (child_info::prefork): Define new function.
+	(frok::child): Clear rd_proc_pipe and wr_proc_pipe so they will not be
+	closed by the child_info destructor.
+	(frok::parent): Use child_info prefork handling, outside of retry loop.
+	Set rd_proc_pipe in child's pinfo after successful CreateProcess.
+	Eliminate postfork call.
+	* globals.cc (my_wr_proc_pipe): Define/declare new variable.
+	* pinfo.cc (pinfo::pending_rd_proc_pipe): Delete.
+	(pinfo::pending_wr_proc_pipe): Ditto.
+	(pinfo::prefork): Ditto.
+	(pinfo::postfork): Ditto.
+	(pinfo::postexec): Ditto.
+	(pinfo::wait): Assume that rd_proc_pipe is set up correctly prior to
+	call.
+	(_pinfo::alert_parent): Replace "wr_proc_pipe" with "my_wr_proc_pipe".
+	* pinfo.h (_pinfo::_wr_proc_pipe): Delete declaration.
+	(_pinfo::set_rd_proc_pipe): Define new function.
+	(pinfo::pending_rd_proc_pipe): Delete declaration.
+	(pinfo::pending_wr_proc_pipe): Ditto.
+	(pinfo::prefork): Ditto.
+	(pinfo::postfork): Ditto.
+	(pinfo::postexec): Ditto.
+	(pinfo::wr_proc_pipe): Ditto.
+	* sigproc.cc (child_info::child_info): Clear rd_proc_pipe and
+	wr_proc_pipe.
+	(child_info::cleanup): Close rd_proc_pipe and
+	wr_proc_pipe if necessary.
+	(child_info_fork::child_info_fork): Set forker_finished to NULL by
+	default.
+	(child_info_spawn::child_info_spawn): Use my_wr_proc_pipe rather than
+	myself->wr_proc_pipe.
+	(child_info::sync): Ditto.
+	(child_info_spawn::cleanup): Call child_info::cleanup.
+	* spawn.cc (child_info_spawn::worker): Remove call to myself.prefork().
+	Set wr_proc_pipe when execing or set up new rd_proc_pipe/wr_proc_pipe
+	via child_info::prefork when spawning.  Remove call to pinfo::postexec.
+	Set rd_proc_pipe in child pinfo when spawning.  Use my_wr_proc_pipe
+	rather than myself->wr_proc_pipe.  Remove call to postfork.
+
 2012-03-19  Christopher Faylor  <me.cygwin2012@cgf.cx>
 
 	* pinfo.cc (pinfo_init): Cosmetic change: unset "destroy" for myself.

winsup/cygwin/child_info.h

@@ -35,7 +35,7 @@ enum child_status
 #define EXEC_MAGIC_SIZE sizeof(child_info)
 
 /* Change this value if you get a message indicating that it is out-of-sync. */
-#define CURR_CHILD_INFO_MAGIC 0x76041b78U
+#define CURR_CHILD_INFO_MAGIC 0xa49e665eU
 
 #define NPROCS	256
 
@@ -61,6 +61,8 @@ public:
   void *cygheap_max;
   unsigned char flag;
   int retry;		// number of times we've tried to start child process
+  HANDLE rd_proc_pipe;
+  HANDLE wr_proc_pipe;
   HANDLE subproc_ready;	// used for synchronization with parent
   HANDLE user_h;
   HANDLE parent;
@@ -78,6 +80,8 @@ public:
   bool isstraced () const {return !!(flag & _CI_STRACED);}
   bool iscygwin () const {return !!(flag & _CI_ISCYGWIN);}
   bool saw_ctrl_c () const {return !!(flag & _CI_SAW_CTRL_C);}
+  void prefork (bool = false);
+  void cleanup ();
 };
 
 class mount_info;

winsup/cygwin/dcrt0.cc

@@ -598,6 +598,17 @@ child_info_fork::handle_fork ()
 	      "user heap", cygheap->user_heap.base, cygheap->user_heap.ptr,
 	      NULL);
 
+  /* If my_wr_proc_pipe != NULL then it's a leftover handle from a previously
+     forked process.  Close it now or suffer confusion with the parent of our
+     parent.  */
+  if (my_wr_proc_pipe)
+    ForceCloseHandle1 (my_wr_proc_pipe, wr_proc_pipe);
+
+  /* Setup our write end of the process pipe.  Clear the one in the structure.
+     The destructor should never be called for this but, it can't hurt to be
+     safe. */
+  my_wr_proc_pipe = wr_proc_pipe;
+  rd_proc_pipe = wr_proc_pipe = NULL;
   /* Do the relocations here.  These will actually likely be overwritten by the
      below child_copy but we do them here in case there is a read-only section
      which does not get copied by fork. */
@@ -626,6 +637,13 @@ child_info_spawn::handle_spawn ()
 			GetCurrentProcess (), &h, 0,
 			FALSE, DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE))
     h = NULL;
+
+  /* Setup our write end of the process pipe.  Clear the one in the structure.
+     The destructor should never be called for this but, it can't hurt to be
+     safe. */
+  my_wr_proc_pipe = wr_proc_pipe;
+  rd_proc_pipe = wr_proc_pipe = NULL;
+
   myself.thisproc (h);
   __argc = moreinfo->argc;
   __argv = moreinfo->argv;

winsup/cygwin/fork.cc

@@ -109,6 +109,25 @@ frok::error (const char *fmt, ...)
   return true;
 }
 
+/* Set up a pipe which will track the life of a "pid" through
+   even after we've exec'ed.  */
+void
+child_info::prefork (bool detached)
+{
+  if (!detached)
+    {
+      if (!CreatePipe (&rd_proc_pipe, &wr_proc_pipe, &sec_none_nih, 16))
+	api_fatal ("prefork: couldn't create pipe process tracker%E");
+
+      if (!SetHandleInformation (wr_proc_pipe, HANDLE_FLAG_INHERIT,
+				 HANDLE_FLAG_INHERIT))
+	api_fatal ("prefork: couldn't set process pipe(%p) inherit state, %E",
+		   wr_proc_pipe);
+      ProtectHandle1 (rd_proc_pipe, rd_proc_pipe);
+      ProtectHandle1 (wr_proc_pipe, wr_proc_pipe);
+    }
+}
+
 int __stdcall
 frok::child (volatile char * volatile here)
 {
@@ -194,6 +213,10 @@ frok::child (volatile char * volatile here)
   ld_preload ();
   fixup_hooks_after_fork ();
   _my_tls.fixup_after_fork ();
+  /* Clear this or the destructor will close them.  In the case of
+     rd_proc_pipe that would be an invalid handle.  In the case of
+     wr_proc_pipe it would be == my_wr_proc_pipe.  Both would be bad. */
+  ch.rd_proc_pipe = ch.wr_proc_pipe = NULL;
   cygwin_finished_initializing = true;
   return 0;
 }
@@ -326,11 +349,11 @@ frok::parent (volatile char * volatile stack_here)
   cygheap->user.deimpersonate ();
   fix_impersonation = true;
   ch.refresh_cygheap ();
+  ch.prefork ();	/* set up process tracking pipes. */
 
   while (1)
     {
       hchild = NULL;
-      myself.prefork ();
       rc = CreateProcessW (myself->progname, /* image to run */
 			   myself->progname, /* what we send in arg0 */
 			   &sec_none_nih,
@@ -403,6 +426,7 @@ frok::parent (volatile char * volatile stack_here)
   /* Fill in fields in the child's process table entry.  */
   child->dwProcessId = pi.dwProcessId;
   child.hProcess = hchild;
+  child.set_rd_proc_pipe (ch.rd_proc_pipe);
 
   /* Hopefully, this will succeed.  The alternative to doing things this
      way is to reserve space prior to calling CreateProcess and then fill
@@ -516,7 +540,6 @@ frok::parent (volatile char * volatile stack_here)
 
 /* Common cleanup code for failure cases */
 cleanup:
-  myself.postfork ();
   if (fix_impersonation)
     cygheap->user.reimpersonate ();
   if (locked)

winsup/cygwin/globals.cc

@@ -21,6 +21,7 @@ details. */
 HANDLE NO_COPY hMainThread;
 HANDLE NO_COPY hProcToken;
 HANDLE NO_COPY hProcImpToken;
+HANDLE my_wr_proc_pipe;
 HMODULE NO_COPY cygwin_hmodule;
 int NO_COPY sigExeced;
 WCHAR windows_system_directory[MAX_PATH];

winsup/cygwin/pinfo.cc

@@ -49,9 +49,6 @@ pinfo_basic myself_initial NO_COPY;
 
 pinfo NO_COPY myself (static_cast<_pinfo *> (&myself_initial));	// Avoid myself != NULL checks
 
-HANDLE NO_COPY pinfo::pending_rd_proc_pipe;
-HANDLE NO_COPY pinfo::pending_wr_proc_pipe;
-
 bool is_toplevel_proc;
 
 /* Setup the pinfo structure for this process.  There may already be a
@@ -989,22 +986,6 @@ debug_printf ("%d exited buf %d\n", vchild->pid, buf);
 bool
 pinfo::wait ()
 {
-  /* If pending_rd_proc_pipe == NULL we're in an execed process which has
-     already grabbed the read end of the pipe from the previous cygwin process
-     running with this pid.  */
-  if (pending_rd_proc_pipe)
-    {
-      /* Our end of the pipe, previously set in prefork() . */
-      rd_proc_pipe = pending_rd_proc_pipe;
-      pending_rd_proc_pipe = NULL;
-
-      /* This sets wr_proc_pipe in the child which, after the following
-	 ForceCloseHandle1, will be only process with the handle open.  */
-      wr_proc_pipe () = pending_wr_proc_pipe;
-      ForceCloseHandle1 (pending_wr_proc_pipe, wr_proc_pipe);
-      pending_wr_proc_pipe = NULL;
-    }
-
   preserve ();		/* Preserve the shared memory associated with the pinfo */
 
   waiter_ready = false;
@@ -1022,54 +1003,6 @@ pinfo::wait ()
   return true;
 }
 
-void
-pinfo::prefork (bool detached)
-{
-  if (wr_proc_pipe () && wr_proc_pipe () != INVALID_HANDLE_VALUE
-      && !SetHandleInformation (wr_proc_pipe (), HANDLE_FLAG_INHERIT, 0))
-    api_fatal ("couldn't set process pipe(%p) inherit state, %E", wr_proc_pipe ());
-  if (!detached)
-    {
-      if (!CreatePipe (&pending_rd_proc_pipe, &pending_wr_proc_pipe,
-		       &sec_none_nih, 16))
-	api_fatal ("Couldn't create pipe tracker for pid %d, %E", (*this)->pid);
-
-      if (!SetHandleInformation (pending_wr_proc_pipe, HANDLE_FLAG_INHERIT,
-				 HANDLE_FLAG_INHERIT))
-	api_fatal ("prefork: couldn't set process pipe(%p) inherit state, %E",
-		   pending_wr_proc_pipe);
-      ProtectHandle1 (pending_rd_proc_pipe, rd_proc_pipe);
-      ProtectHandle1 (pending_wr_proc_pipe, wr_proc_pipe);
-    }
-}
-
-void
-pinfo::postfork ()
-{
-  if (wr_proc_pipe () && wr_proc_pipe () != INVALID_HANDLE_VALUE
-      && !SetHandleInformation (wr_proc_pipe (), HANDLE_FLAG_INHERIT,
-			       HANDLE_FLAG_INHERIT))
-    api_fatal ("postfork: couldn't set process pipe(%p) inherit state, %E", wr_proc_pipe ());
-  if (pending_rd_proc_pipe)
-    {
-      ForceCloseHandle1 (pending_rd_proc_pipe, rd_proc_pipe);
-      pending_rd_proc_pipe = NULL;
-    }
-  if (pending_wr_proc_pipe)
-    {
-      ForceCloseHandle1 (pending_wr_proc_pipe, wr_proc_pipe);
-      pending_wr_proc_pipe = NULL;
-    }
-}
-
-void
-pinfo::postexec ()
-{
-  if (wr_proc_pipe () && wr_proc_pipe () != INVALID_HANDLE_VALUE
-      && !ForceCloseHandle1 (wr_proc_pipe (), wr_proc_pipe))
-    api_fatal ("postexec: couldn't close wr_proc_pipe(%p), %E", wr_proc_pipe ());
-}
-
 /* function to send a "signal" to the parent when something interesting happens
    in the child. */
 bool
@@ -1082,19 +1015,17 @@ _pinfo::alert_parent (char sig)
 
      FIXME: Is there a race here if we run this while another thread is attempting
      to exec()? */
-  if (wr_proc_pipe == INVALID_HANDLE_VALUE || !myself.wr_proc_pipe () || have_execed)
+  if (my_wr_proc_pipe)
-    /* no parent */;
-  else
     {
-      if (WriteFile (wr_proc_pipe, &sig, 1, &nb, NULL))
+      if (WriteFile (my_wr_proc_pipe, &sig, 1, &nb, NULL))
 	/* all is well */;
       else if (GetLastError () != ERROR_BROKEN_PIPE)
 	debug_printf ("sending %d notification to parent failed, %E", sig);
       else
 	{
 	  ppid = 1;
-	  HANDLE closeit = wr_proc_pipe;
+	  HANDLE closeit = my_wr_proc_pipe;
-	  wr_proc_pipe = INVALID_HANDLE_VALUE;
+	  my_wr_proc_pipe = NULL;
 	  ForceCloseHandle1 (closeit, wr_proc_pipe);
 	}
     }

winsup/cygwin/pinfo.h

@@ -121,7 +121,6 @@ public:
   HANDLE exec_sendsig;
   DWORD exec_dwProcessId;
 public:
-  HANDLE wr_proc_pipe;
   friend class pinfo_minimal;
 };
 
@@ -140,6 +139,7 @@ public:
   HANDLE hProcess;
   HANDLE rd_proc_pipe;
   pinfo_minimal (): h (NULL), hProcess (NULL), rd_proc_pipe (NULL) {}
+  void set_rd_proc_pipe (HANDLE& h) {rd_proc_pipe = h; h = NULL;}
   friend class pinfo;
 };
 
@@ -147,8 +147,6 @@ class pinfo: public pinfo_minimal
 {
   bool destroy;
   _pinfo *procinfo;
-  static HANDLE pending_rd_proc_pipe;
-  static HANDLE pending_wr_proc_pipe;
 public:
   bool waiter_ready;
   class cygthread *wait_thread;
@@ -204,15 +202,10 @@ public:
     return res;
   }
 #endif
-  void prefork (bool = false);
-  void postfork ();
-  void postexec ();
   HANDLE shared_handle () {return h;}
   void set_acl ();
   friend class _pinfo;
   friend class winpids;
-private:
-  HANDLE& wr_proc_pipe() {return procinfo->wr_proc_pipe;}
 };
 
 #define ISSTATE(p, f)	(!!((p)->process_state & f))

winsup/cygwin/sigproc.cc

@@ -828,7 +828,8 @@ child_info::child_info (unsigned in_cb, child_info_types chtype,
 			bool need_subproc_ready):
   cb (in_cb), intro (PROC_MAGIC_GENERIC), magic (CHILD_INFO_MAGIC),
   type (chtype), cygheap (::cygheap), cygheap_max (::cygheap_max),
-  flag (0), retry (child_info::retry_count)
+  flag (0), retry (child_info::retry_count), rd_proc_pipe (NULL),
+  wr_proc_pipe (NULL)
 {
   /* It appears that when running under WOW64 on Vista 64, the first DWORD
      value in the datastructure lpReserved2 is pointing to (msv_count in
@@ -876,14 +877,12 @@ child_info::child_info (unsigned in_cb, child_info_types chtype,
 
 child_info::~child_info ()
 {
-  if (subproc_ready)
+  cleanup ();
-    CloseHandle (subproc_ready);
-  if (parent)
-    CloseHandle (parent);
 }
 
 child_info_fork::child_info_fork () :
-  child_info (sizeof *this, _CH_FORK, true)
+  child_info (sizeof *this, _CH_FORK, true),
+  forker_finished (NULL)
 {
 }
 
@@ -893,7 +892,7 @@ child_info_spawn::child_info_spawn (child_info_types chtype, bool need_subproc_r
   if (type == _CH_EXEC)
     {
       hExeced = NULL;
-      if (myself->wr_proc_pipe)
+      if (my_wr_proc_pipe)
 	ev = NULL;
       else if (!(ev = CreateEvent (&sec_none_nih, false, false, NULL)))
 	api_fatal ("couldn't create signalling event for exec, %E");
@@ -912,6 +911,31 @@ cygheap_exec_info::alloc ()
 					     + (nprocs * sizeof (children[0])));
 }
 
+void
+child_info::cleanup ()
+{
+  if (subproc_ready)
+    {
+      CloseHandle (subproc_ready);
+      subproc_ready = NULL;
+    }
+  if (parent)
+    {
+      CloseHandle (parent);
+      parent = NULL;
+    }
+  if (rd_proc_pipe)
+    {
+      ForceCloseHandle (rd_proc_pipe);
+      rd_proc_pipe = NULL;
+    }
+  if (wr_proc_pipe)
+    {
+      ForceCloseHandle (wr_proc_pipe);
+      wr_proc_pipe = NULL;
+    }
+}
+
 void
 child_info_spawn::cleanup ()
 {
@@ -940,6 +964,7 @@ child_info_spawn::cleanup ()
       sync_proc_subproc.release ();
     }
   type = _CH_NADA;
+  child_info::cleanup ();
 }
 
 /* Record any non-reaped subprocesses to be passed to about-to-be-execed
@@ -1047,7 +1072,7 @@ child_info::sync (pid_t pid, HANDLE& hProcess, DWORD howlong)
 	{
 	  res = true;
 	  exit_code = STILL_ACTIVE;
-	  if (type == _CH_EXEC && myself->wr_proc_pipe)
+	  if (type == _CH_EXEC && my_wr_proc_pipe)
 	    {
 	      ForceCloseHandle1 (hProcess, childhProc);
 	      hProcess = NULL;

winsup/cygwin/spawn.cc

@@ -352,10 +352,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
   if (mode == _P_OVERLAY)
     chtype = _CH_EXEC;
   else
-    {
+    chtype = _CH_SPAWN;
-      chtype = _CH_SPAWN;
-      myself.prefork ();
-    }
 
   moreinfo = cygheap_exec_info::alloc ();
 
@@ -616,6 +613,12 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
       && fhandler_console::tc_getpgid () != myself->pgid)
     c_flags |= CREATE_NEW_PROCESS_GROUP;
   refresh_cygheap ();
+
+  if (chtype == _CH_EXEC)
+    wr_proc_pipe = my_wr_proc_pipe;
+  else
+    prefork ();
+
   /* When ruid != euid we create the new process under the current original
      account and impersonate in child, this way maintaining the different
      effective vs. real ids.
@@ -737,6 +740,8 @@ loop:
 	  myself->exec_sendsig = NULL;
 	}
       myself->process_state &= ~PID_NOTCYGWIN;
+      if (wr_proc_pipe == my_wr_proc_pipe)
+	wr_proc_pipe = NULL;	/* We still own it: don't nuke in destructor */
       res = -1;
       goto out;
     }
@@ -771,18 +776,6 @@ loop:
       myself.hProcess = hExeced = pi.hProcess;
       real_path.get_wide_win32_path (myself->progname); // FIXME: race?
       sigproc_printf ("new process name %W", myself->progname);
-      /* If wr_proc_pipe doesn't exist then this process was not started by a cygwin
-	 process.  So, we need to wait around until the process we've just "execed"
-	 dies.  Use our own wait facility below to wait for our own pid to exit
-	 (there is some minor special case code in proc_waiter and friends to
-	 accommodate this).
-
-	 If wr_proc_pipe exists, then it will be inherited by the child.
-	 If the child has exited already, that's ok.  The parent will pick up
-	 on this fact when we exit.  myself.postexec () will close our end of
-	 the pipe.  */
-      if (!newargv.win16_exe && myself->wr_proc_pipe)
-	myself.postexec ();
       pid = myself->pid;
       if (!iscygwin ())
 	close_all_files ();
@@ -801,6 +794,7 @@ loop:
 	  res = -1;
 	  goto out;
 	}
+      child.set_rd_proc_pipe (rd_proc_pipe);
       child->dwProcessId = pi.dwProcessId;
       child.hProcess = pi.hProcess;
 
@@ -857,7 +851,7 @@ loop:
       else
 	{
 	  close_all_files (true);
-	  if (!myself->wr_proc_pipe
+	  if (!my_wr_proc_pipe
 	      && WaitForSingleObject (pi.hProcess, 0) == WAIT_TIMEOUT)
 	    {
 	      extern bool is_toplevel_proc;
@@ -887,8 +881,6 @@ loop:
     }
 
 out:
-  if (mode != _P_OVERLAY)
-    myself.postfork ();
   this->cleanup ();
   if (envblock)
     free (envblock);