* DevNotes: Add entry cgf-000004.

* pinfo.cc (pinfo::init): Reuse shared memory if the state is marked with
PID_REAPED.
* spawn.cc (child_info_spawn::worker): Don't duplicate myself_pinfo into
non-cygwin child.
* fork.cc (frok::parent): Improve error output.

5 files changed, 60 insertions, 8 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 0e3056e..760df76 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,13 @@
+2012-05-08  Christopher Faylor  <me.cygwin2012@cgf.cx>
+
+	* DevNotes: Add entry cgf-000004.
+	* pinfo.cc (pinfo::init): Reuse shared memory if the state is marked
+	with PID_REAPED.
+	* spawn.cc (child_info_spawn::worker): Don't duplicate myself_pinfo
+	into non-cygwin child.
+
+	* fork.cc (frok::parent): Improve error output.
+
 2012-05-07  Christopher Faylor  <me.cygwin2012@cgf.cx>
 
 	* DevNotes: Add entry cgf-000003.
diff --git a/winsup/cygwin/DevNotes b/winsup/cygwin/DevNotes
index b9d031f..83be5e9 100644
--- a/winsup/cygwin/DevNotes
+++ b/winsup/cygwin/DevNotes
@@ -1,4 +1,39 @@
-2012-05-03  cgf-000003
+2012-05-08  cgf-000004
+
+The change associated with cgf-000003 introduced a new problem.
+
+Since a handle associated with the parent is no longer being duplicated
+into a non-cygwin "execed child", Windows is free to reuse the pid of
+the parent when the parent exits.  However, since we *did* duplicate a
+handle pointing to the pid's shared memory area into the "execed child",
+the shared memory for the pid was still active.
+
+Since the shared memory was still available, if a new process reuses the
+previous pid, Cygwin would detect that the shared memory was not created
+and had a "PID_REAPED" flag.  That was considered an error, and, so, it
+would set procinfo to NULL and pinfo::thisproc would die since this
+situation is not supposed to occur.
+
+I fixed this in two ways:
+
+1) If a shared memory region has a PID_REAPED flag then zero it and
+reuse it.  This should be safe since you are not really supposed to be
+querying the shared memory region for anything after PID_REAPED has been
+set.
+
+2) Forego duping a copy of myself_pinfo if we're starting a non-cygwin
+child for exec.
+
+It seems like 2) is a common theme and an audit of all of the handles
+that are being passed to non-cygwin children is in order for 1.7.16.
+
+The other minor modification that was made in this change was to add the
+pid of the failing process to fork error output.  This helps slightly
+when looking at strace output, even though in this case it was easy to
+find what was failing by looking for '^---' when running the "stv"
+strace dumper.  That found the offending exception quickly.
+
+2012-05-07  cgf-000003
 
 <1.7.15>
 Don't make Cygwin wait for all children of a non-cygwin child program.
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index e617bd1..22076fc 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -393,8 +393,8 @@ frok::parent (volatile char * volatile stack_here)
       /* Wait for subproc to initialize itself. */
       if (!ch.sync (pi.dwProcessId, hchild, FORK_WAIT_TIMEOUT))
 	{
-	  if (!error ("forked process died unexpectedly, retry %d, exit code %d",
-		      ch.retry, ch.exit_code))
+	  if (!error ("forked process %u died unexpectedly, retry %d, exit code %d",
+		      pi.dwProcessId, ch.retry, ch.exit_code))
 	    continue;
 	  this_errno = EAGAIN;
 	  goto cleanup;
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index daed1cc..fda0abc 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -300,6 +300,13 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
 
       bool created = shloc != SH_JUSTOPEN;
 
+      if (!created && createit && (procinfo->process_state & PID_REAPED))
+	{
+	  memset (procinfo, 0, sizeof (*procinfo));
+	  created = true;	/* Lie that we created this - just reuse old
+				   shared memory */
+	} 
+
       if ((procinfo->process_state & PID_REAPED)
 	  || ((procinfo->process_state & PID_INITIALIZING) && (flag & PID_NOREDIR)
 	      && cygwin_pid (procinfo->dwProcessId) != procinfo->pid))
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 8b5e382..7e165cf 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -422,10 +422,10 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
       moreinfo->argc = newargv.argc;
       moreinfo->argv = newargv;
 
-      if (mode != _P_OVERLAY ||
-	  !DuplicateHandle (GetCurrentProcess (), myself.shared_handle (),
-			    GetCurrentProcess (), &moreinfo->myself_pinfo,
-			    0, TRUE, DUPLICATE_SAME_ACCESS))
+      if (mode != _P_OVERLAY || !real_path.iscygexec ()
+	  || !DuplicateHandle (GetCurrentProcess (), myself.shared_handle (),
+			       GetCurrentProcess (), &moreinfo->myself_pinfo,
+			       0, TRUE, DUPLICATE_SAME_ACCESS))
 	moreinfo->myself_pinfo = NULL;
       else
 	VerifyHandle (moreinfo->myself_pinfo);
@@ -740,7 +740,7 @@ loop:
 	 process fails.  Only need to do this for _P_OVERLAY since the handle will
 	 be closed otherwise.  Don't need to do this for 'parent' since it will
          be closed in every case.  See FIXME above. */
-      if (!real_path.iscygexec () && mode == _P_OVERLAY)
+      if (!iscygwin () && mode == _P_OVERLAY)
 	SetHandleInformation (wr_proc_pipe, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT);
       if (wr_proc_pipe == my_wr_proc_pipe)
 	wr_proc_pipe = NULL;	/* We still own it: don't nuke in destructor */