* fork.cc (lock_signals): Move to sigproc.h.

(lock_pthread): Ditto.
(hold_everything): Ditto.
(frok::parent): Call myself.prefork() just before calling CreateProcess.  Call
myself.postfork () on function exit.
* pinfo.cc (pinfo::pending_rd_proc_pipe): Define.
(pinfo::pending_wr_proc_pipe): Ditto.
(_pinfo::dup_proc_pipe): Delete.
(pinfo::wait): Move pipe creation into pinfo::prefork.  Set pipe variables from
pending_*.
(_pinfo::sync_proc_pipe): Delete.
(_pinfo::proc_pipe_owner): Ditto.
(pinfo::prefork): Define new function.
(pinfo::postfork): Ditto.
(pinfo::postexec): Ditto.
(_pinfo::alert_parent): Remove obsolete call to sync_proc_pipe.
(_pinfo::dup_proc_pipe): Delete declaration.
(_pinfo::sync_proc_pipe): Ditto.
(pinfo::pending_rd_proc_pipe): Declare.
(pinfo::pending_wr_proc_pipe): Ditto.
(pinfo::prefork): Declare new function.
(pinfo::postfork): Ditto.
(pinfo::postexec): Ditto.
(pinfo::wr_proc_pipe): Define new wrapper function.
* sigproc.h: Include "sync.h".  Move locking functions from fork to here.
* spawn.cc (child_info_spawn::worker): Delete now-unneeded requirement to
record orig_wr_proc_pipe.  Call hold_everything prior to doing anything.  Call
myself.prefork() if spawning.  Replace wr_proc_pipe synchronization with call
to myself.postexec().  Call myself.postfork() if not execing.
* sync.h: Replace #ifdef wrapper with "#pragma once".

7 files changed, 192 insertions, 169 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index d0f74cf61..ebcf00afd 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,38 @@
+2012-03-16  Christopher Faylor  <me.cygwin2012@cgf.cx>
+
+	* fork.cc (lock_signals): Move to sigproc.h.
+	(lock_pthread): Ditto.
+	(hold_everything): Ditto.
+	(frok::parent): Call myself.prefork() just before calling
+	CreateProcess.  Call myself.postfork () on function exit.
+	* pinfo.cc (pinfo::pending_rd_proc_pipe): Define.
+	(pinfo::pending_wr_proc_pipe): Ditto.
+	(_pinfo::dup_proc_pipe): Delete.
+	(pinfo::wait): Move pipe creation into pinfo::prefork.  Set pipe
+	variables from pending_*.
+	(_pinfo::sync_proc_pipe): Delete.
+	(_pinfo::proc_pipe_owner): Ditto.
+	(pinfo::prefork): Define new function.
+	(pinfo::postfork): Ditto.
+	(pinfo::postexec): Ditto.
+	(_pinfo::alert_parent): Remove obsolete call to sync_proc_pipe.
+	(_pinfo::dup_proc_pipe): Delete declaration.
+	(_pinfo::sync_proc_pipe): Ditto.
+	(pinfo::pending_rd_proc_pipe): Declare.
+	(pinfo::pending_wr_proc_pipe): Ditto.
+	(pinfo::prefork): Declare new function.
+	(pinfo::postfork): Ditto.
+	(pinfo::postexec): Ditto.
+	(pinfo::wr_proc_pipe): Define new wrapper function.
+	* sigproc.h: Include "sync.h".  Move locking functions from fork to
+	here.
+	* spawn.cc (child_info_spawn::worker): Delete now-unneeded requirement
+	to record orig_wr_proc_pipe.  Call hold_everything prior to doing
+	anything.  Call myself.prefork() if spawning.  Replace wr_proc_pipe
+	synchronization with call to myself.postexec().  Call myself.postfork()
+	if not execing.
+	* sync.h: Replace #ifdef wrapper with "#pragma once".
+
 2012-03-13  Corinna Vinschen  <corinna@vinschen.de>
 
 	* hookapi.cc (hook_or_detect_cygwin): Change condition when to use
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 56ba0fc0f..2b29a1d70 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -47,83 +47,6 @@ class frok
   friend int fork ();
 };
 
-class lock_signals
-{
-  bool worked;
-public:
-  lock_signals ()
-  {
-    worked = sig_send (NULL, __SIGHOLD) == 0;
-  }
-  operator int () const
-  {
-    return worked;
-  }
-  void dont_bother ()
-  {
-    worked = false;
-  }
-  ~lock_signals ()
-  {
-    if (worked)
-      sig_send (NULL, __SIGNOHOLD);
-  }
-};
-
-class lock_pthread
-{
-  bool bother;
-public:
-  lock_pthread (): bother (1)
-  {
-    pthread::atforkprepare ();
-  }
-  void dont_bother ()
-  {
-    bother = false;
-  }
-  ~lock_pthread ()
-  {
-    if (bother)
-      pthread::atforkparent ();
-  }
-};
-
-class hold_everything
-{
-public: /* DELETEME*/
-  bool& ischild;
-  /* Note the order of the locks below.  It is important,
-     to avoid races, that the lock order be preserved.
-
-     pthread is first because it serves as a master lock
-     against other forks being attempted while this one is active.
-
-     signals is next to stop signal processing for the duration
-     of the fork.
-
-     process is last.  If it is put before signals, then a deadlock
-     could be introduced if the process attempts to exit due to a signal. */
-  lock_pthread pthread;
-  lock_signals signals;
-  lock_process process;
-
-public:
-  hold_everything (bool& x): ischild (x) {}
-  operator int () const {return signals;}
-
-  ~hold_everything()
-  {
-    if (ischild)
-      {
-	pthread.dont_bother ();
-	process.dont_bother ();
-	signals.dont_bother ();
-      }
-  }
-
-};
-
 static void
 resume_child (HANDLE forker_finished)
 {
@@ -407,6 +330,7 @@ frok::parent (volatile char * volatile stack_here)
   while (1)
     {
       hchild = NULL;
+      myself.prefork ();
       rc = CreateProcessW (myself->progname, /* image to run */
 			   myself->progname, /* what we send in arg0 */
 			   &sec_none_nih,
@@ -592,6 +516,7 @@ frok::parent (volatile char * volatile stack_here)
 
 /* Common cleanup code for failure cases */
 cleanup:
+  myself.postfork ();
   if (fix_impersonation)
     cygheap->user.reimpersonate ();
   if (locked)
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index d7e147ae5..c5e646455 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -49,6 +49,9 @@ 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
@@ -980,66 +983,16 @@ proc_waiter (void *arg)
   return 0;
 }
 
-#ifdef DEBUGGING
-#define warn_printf api_fatal
-#else
-#define warn_printf system_printf
-#endif
-HANDLE
-_pinfo::dup_proc_pipe (HANDLE hProcess, const char *func)
-{
-  DWORD flags = DUPLICATE_SAME_ACCESS;
-  HANDLE orig_wr_proc_pipe = wr_proc_pipe;
-  /* Can't set DUPLICATE_CLOSE_SOURCE for exec case because we could be
-     execing a non-cygwin process and we need to set the exit value before the
-     parent sees it.  */
-  if (this != myself || is_toplevel_proc)
-    flags |= DUPLICATE_CLOSE_SOURCE;
-  bool res = DuplicateHandle (GetCurrentProcess (), wr_proc_pipe,
-			      hProcess, &wr_proc_pipe, 0, FALSE, flags);
-  if (res)
-    {
-      wr_proc_pipe_owner = dwProcessId;
-      sigproc_printf ("(%s) duped wr_proc_pipe %p for pid %d(%u)", func,
-		      wr_proc_pipe, pid, dwProcessId);
-    }
-  else
-    {
-      DWORD duperr = GetLastError ();
-      DWORD wfsores = WaitForSingleObject (hProcess, 0);
-      if (wfsores != WAIT_OBJECT_0)
-	{
-	  warn_printf ("(%s) process synchronization failed for pid %u/%p, "
-		       "wr_proc_pipe %p vs. %p: DuplicateHandle winerr %d, "
-		       "WFSO returned %u, %E",
-		       func, pid, hProcess, wr_proc_pipe, orig_wr_proc_pipe, duperr,
-		       wfsores);
-	}
-      wr_proc_pipe = orig_wr_proc_pipe;
-    }
-  return orig_wr_proc_pipe;
-}
-
 /* function to set up the process pipe and kick off proc_waiter */
 bool
 pinfo::wait ()
 {
-  /* If rd_proc_pipe != NULL we're in an execed process which already has
-     grabbed the read end of the pipe from the previous cygwin process running
-     with this pid.  */
-  if (!rd_proc_pipe)
-    {
-      /* FIXME: execed processes should be able to wait for pids that were started
-	 by the process which execed them. */
-      if (!CreatePipe (&rd_proc_pipe, &((*this)->wr_proc_pipe), &sec_none_nih, 16))
-	{
-	  system_printf ("Couldn't create pipe tracker for pid %d, %E",
-			 (*this)->pid);
-	  return false;
-	}
+  rd_proc_pipe = pending_rd_proc_pipe;
+  pending_rd_proc_pipe = NULL;
 
-      (*this)->dup_proc_pipe (hProcess, "pinfo::wait");
-    }
+  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 */
 
@@ -1059,11 +1012,48 @@ pinfo::wait ()
 }
 
 void
-_pinfo::sync_proc_pipe ()
+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 rd_proc_pipe != NULL we're in an execed process which already has
+     grabbed the read end of the pipe from the previous cygwin process running
+     with this pid.  */
+  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);
+  if (pending_wr_proc_pipe)
+    ForceCloseHandle1 (pending_wr_proc_pipe, wr_proc_pipe);
+}
+
+void
+pinfo::postexec ()
 {
-  if (wr_proc_pipe && wr_proc_pipe != INVALID_HANDLE_VALUE)
-    while (wr_proc_pipe_owner != GetCurrentProcessId ())
-      yield ();
+  if (wr_proc_pipe () && wr_proc_pipe () != INVALID_HANDLE_VALUE
+      && !ForceCloseHandle (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
@@ -1078,11 +1068,10 @@ _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 (wr_proc_pipe == INVALID_HANDLE_VALUE || !myself.wr_proc_pipe () || have_execed)
     /* no parent */;
   else
     {
-      sync_proc_pipe ();
       if (WriteFile (wr_proc_pipe, &sig, 1, &nb, NULL))
 	/* all is well */;
       else if (GetLastError () != ERROR_BROKEN_PIPE)
diff --git a/winsup/cygwin/pinfo.h b/winsup/cygwin/pinfo.h
index 551f3f29c..1e514e3c0 100644
--- a/winsup/cygwin/pinfo.h
+++ b/winsup/cygwin/pinfo.h
@@ -111,8 +111,6 @@ public:
   char *cwd (size_t &);
   char *cmdline (size_t &);
   bool set_ctty (class fhandler_termios *, int);
-  HANDLE dup_proc_pipe (HANDLE, const char *) __attribute__ ((regparm(3)));
-  void sync_proc_pipe ();
   bool alert_parent (char);
   int __stdcall kill (siginfo_t&) __attribute__ ((regparm (2)));
   bool __stdcall exists () __attribute__ ((regparm (1)));
@@ -124,7 +122,6 @@ public:
   DWORD exec_dwProcessId;
 public:
   HANDLE wr_proc_pipe;
-  DWORD wr_proc_pipe_owner;
   friend class pinfo_minimal;
 };
 
@@ -150,6 +147,8 @@ 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;
@@ -205,10 +204,15 @@ 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))
diff --git a/winsup/cygwin/sigproc.h b/winsup/cygwin/sigproc.h
index 2900da4cc..ffe75a8d3 100644
--- a/winsup/cygwin/sigproc.h
+++ b/winsup/cygwin/sigproc.h
@@ -11,6 +11,7 @@ details. */
 
 #pragma once
 #include <signal.h>
+#include "sync.h"
 
 #ifdef NSIG
 enum
@@ -121,4 +122,81 @@ extern char myself_nowait_dummy[];
 
 extern struct sigaction *global_sigs;
 
+class lock_signals
+{
+  bool worked;
+public:
+  lock_signals ()
+  {
+    worked = sig_send (NULL, __SIGHOLD) == 0;
+  }
+  operator int () const
+  {
+    return worked;
+  }
+  void dont_bother ()
+  {
+    worked = false;
+  }
+  ~lock_signals ()
+  {
+    if (worked)
+      sig_send (NULL, __SIGNOHOLD);
+  }
+};
+
+class lock_pthread
+{
+  bool bother;
+public:
+  lock_pthread (): bother (1)
+  {
+    pthread::atforkprepare ();
+  }
+  void dont_bother ()
+  {
+    bother = false;
+  }
+  ~lock_pthread ()
+  {
+    if (bother)
+      pthread::atforkparent ();
+  }
+};
+
+class hold_everything
+{
+public: /* DELETEME*/
+  bool ischild;
+  /* Note the order of the locks below.  It is important,
+     to avoid races, that the lock order be preserved.
+
+     pthread is first because it serves as a master lock
+     against other forks being attempted while this one is active.
+
+     signals is next to stop signal processing for the duration
+     of the fork.
+
+     process is last.  If it is put before signals, then a deadlock
+     could be introduced if the process attempts to exit due to a signal. */
+  lock_pthread pthread;
+  lock_signals signals;
+  lock_process process;
+
+public:
+  hold_everything (bool x = false): ischild (x) {}
+  operator int () const {return signals;}
+
+  ~hold_everything()
+  {
+    if (ischild)
+      {
+	pthread.dont_bother ();
+	process.dont_bother ();
+	signals.dont_bother ();
+      }
+  }
+
+};
+
 #define myself_nowait ((_pinfo *) myself_nowait_dummy)
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index c2412305a..ef0403ec0 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -308,6 +308,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
       return -1;
     }
 
+  hold_everything for_now;
   /* FIXME: There is a small race here and FIXME: not thread safe! */
 
   pthread_cleanup cleanup;
@@ -335,7 +336,6 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
   bool null_app_name = false;
   STARTUPINFOW si = {};
   int looped = 0;
-  HANDLE orig_wr_proc_pipe = NULL;
 
   myfault efault;
   if (efault.faulted ())
@@ -349,10 +349,13 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
     }
 
   child_info_types chtype;
-  if (mode != _P_OVERLAY)
-    chtype = _CH_SPAWN;
-  else
+  if (mode == _P_OVERLAY)
     chtype = _CH_EXEC;
+  else
+    {
+      chtype = _CH_SPAWN;
+      myself.prefork ();
+    }
 
   moreinfo = cygheap_exec_info::alloc ();
 
@@ -770,23 +773,16 @@ loop:
       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 to wait for our own pid to exit (there
-	 is some minor special case code in proc_waiter and friends to accommodate
-	 this).
+	 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 should be duplicated to the child.
+	 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.  dup_proc_pipe will close our end of the pipe.
-	 Note that wr_proc_pipe may also be == INVALID_HANDLE_VALUE.  That will make
-	 dup_proc_pipe essentially a no-op.  */
+	 on this fact when we exit.  myself.postexec () will close our end of
+	 the pipe.  */
       if (!newargv.win16_exe && myself->wr_proc_pipe)
-	{
-	  if (!looped)
-	    myself->sync_proc_pipe ();	/* Make sure that we own wr_proc_pipe
-					   just in case we've been previously
-					   execed. */
-	  orig_wr_proc_pipe = myself->dup_proc_pipe (pi.hProcess, "child_info_spawn::worker");
-	}
+	myself.postexec ();
       pid = myself->pid;
       if (!iscygwin ())
 	close_all_files ();
@@ -851,11 +847,6 @@ loop:
       myself.hProcess = pi.hProcess;
       if (!synced)
 	{
-	  if (orig_wr_proc_pipe)
-	    {
-	      myself->wr_proc_pipe_owner = GetCurrentProcessId ();
-	      myself->wr_proc_pipe = orig_wr_proc_pipe;
-	    }
 	  if (!proc_retry (pi.hProcess))
 	    {
 	      looped++;
@@ -896,6 +887,8 @@ loop:
     }
 
 out:
+  if (mode != _P_OVERLAY)
+    myself.postfork ();
   this->cleanup ();
   if (envblock)
     free (envblock);
diff --git a/winsup/cygwin/sync.h b/winsup/cygwin/sync.h
index 2215599c8..c9c5fb595 100644
--- a/winsup/cygwin/sync.h
+++ b/winsup/cygwin/sync.h
@@ -1,6 +1,7 @@
 /* sync.h: Header file for cygwin synchronization primitives.
 
-   Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007 Red Hat, Inc.
+   Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2011, 2012
+   Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -8,8 +9,8 @@ This software is a copyrighted work licensed under the terms of the
 Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
 details. */
 
-#ifndef _SYNC_H
-#define _SYNC_H
+#pragma once
+
 /* FIXME: Note that currently this class cannot be allocated via `new' since
    there are issues with malloc and fork. */
 class muto
@@ -62,5 +63,3 @@ public:
   friend class dtable;
   friend class fhandler_fifo;
 };
-
-#endif /*_SYNC_H*/