* pinfo.h (maybe_set_exit_code_from_windows): Renamed from set_exit_state.

* pinfo.cc (pinfo::exit): Use renamed function.
(proc_waiter): Ditto.  Make a copy of input argument to avoid problems when
procs array is shuffled.  Flag when copy is made so that remove_proc knows when
it is safe to reshuffle.
* sigproc.cc (proc_terminate): Don't flag process_state as PID_EXITED.
(remove_proc): Wait for waiter to finish copying pinfo element before moving it
(an actual wait should be an extremely rare event).

4 files changed, 35 insertions, 11 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index e48bd37..ae1a2f7 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,15 @@
+2005-01-16  Christopher Faylor  <cgf@timesys.com>
+
+	* pinfo.h (maybe_set_exit_code_from_windows): Renamed from
+	set_exit_state.
+	* pinfo.cc (pinfo::exit): Use renamed function.
+	(proc_waiter): Ditto.  Make a copy of input argument to avoid problems
+	when procs array is shuffled.  Flag when copy is made so that
+	remove_proc knows when it is safe to reshuffle.
+	* sigproc.cc (proc_terminate): Don't flag process_state as PID_EXITED.
+	(remove_proc): Wait for waiter to finish copying pinfo element before
+	moving it (an actual wait should be an extremely rare event).
+
 2005-01-15  Christopher Faylor  <cgf@timesys.com>
 
 	* init.cc (dll_entry): Remove unused extern.
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 4659e37..436785d 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -105,7 +105,7 @@ pinfo_init (char **envp, int envc)
 
 # define self (*this)
 void
-pinfo::set_exit_state ()
+pinfo::maybe_set_exit_code_from_windows ()
 {
   DWORD x = 0xdeadbeef;
   DWORD oexitcode = self->exitcode;
@@ -135,7 +135,7 @@ pinfo::exit (DWORD n)
   fill_rusage (&r, hMainProc);
   add_rusage (&self->rusage_self, &r);
 
-  set_exit_state ();
+  maybe_set_exit_code_from_windows ();
   if (n != EXITCODE_NOSET)
     self->alert_parent (0);
   int exitcode = self->exitcode;
@@ -675,7 +675,8 @@ _pinfo::cmdline (size_t& n)
 static DWORD WINAPI
 proc_waiter (void *arg)
 {
-  pinfo& vchild = *(pinfo *) arg;
+  pinfo vchild = *(pinfo *) arg;
+  ((pinfo *) arg)->waiter_ready = true;
 
   siginfo_t si;
   si.si_signo = SIGCHLD;
@@ -714,8 +715,7 @@ proc_waiter (void *arg)
 	  /* Child exited.  Do some cleanup and signal myself.  */
 	  CloseHandle (vchild.rd_proc_pipe);
 	  vchild.rd_proc_pipe = NULL;
-	  vchild.set_exit_state ();
-	  vchild->process_state = PID_EXITED;
+	  vchild.maybe_set_exit_code_from_windows ();
 	  if (WIFEXITED (vchild->exitcode))
 	    si.si_sigval.sival_int = CLD_EXITED;
 	  else if (WCOREDUMP (vchild->exitcode))
@@ -723,6 +723,8 @@ proc_waiter (void *arg)
 	  else
 	    si.si_sigval.sival_int = CLD_KILLED;
 	  si.si_status = vchild->exitcode;
+	  vchild->process_state = PID_EXITED;
+	  /* This should always be last.  Do not use vchild-> beyond this point */
 	  break;
 	case SIGTTIN:
 	case SIGTTOU:
@@ -812,6 +814,7 @@ pinfo::wait ()
 
   preserve ();		/* Preserve the shared memory associated with the pinfo */
 
+  waiter_ready = false;
   /* Fire up a new thread to track the subprocess */
   cygthread *h = new cygthread (proc_waiter, this, "proc_waiter");
   if (!h)
@@ -854,6 +857,7 @@ _pinfo::alert_parent (char sig)
 	debug_printf ("sending %d notification to parent failed, %E", sig);
       else
 	{
+	  ppid = 1;
 	  HANDLE closeit = wr_proc_pipe;
 	  wr_proc_pipe = INVALID_HANDLE_VALUE;
 	  CloseHandle (closeit);
diff --git a/winsup/cygwin/pinfo.h b/winsup/cygwin/pinfo.h
index fc29139..d202d91 100644
--- a/winsup/cygwin/pinfo.h
+++ b/winsup/cygwin/pinfo.h
@@ -140,6 +140,7 @@ public:
   HANDLE rd_proc_pipe;
   HANDLE hProcess;
   CRITICAL_SECTION _lock;
+  bool waiter_ready;
   /* Handle associated with initial Windows pid which started it all. */
   class cygthread *wait_thread;
   void init (pid_t, DWORD, HANDLE = NULL) __attribute__ ((regparm(3)));
@@ -155,7 +156,7 @@ public:
       release ();
   }
   void exit (DWORD n) __attribute__ ((noreturn, regparm(2)));
-  void set_exit_state () __attribute__ ((regparm(2)));
+  void maybe_set_exit_code_from_windows () __attribute__ ((regparm(1)));
   void initialize_lock () {InitializeCriticalSection (&_lock);}
   void lock () {EnterCriticalSection (&_lock);}
   void unlock () {LeaveCriticalSection (&_lock);}
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index fb06808..b99a90c 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -65,8 +65,9 @@ Static HANDLE wait_sig_inited;		// Control synchronization of
 					//  message queue startup
 
 Static int nprocs;			// Number of deceased children
-Static char cprocs[(NPROCS + 1) * sizeof (pinfo)];		// All my deceased children info
-#define procs ((pinfo *) cprocs)
+Static char cprocs[(NPROCS + 1) * sizeof (pinfo)];// All my children info
+#define procs ((pinfo *) cprocs)	// All this just to avoid expensive
+					// constructor operation  at DLL startup
 Static waitq waitq_head = {0, 0, 0, 0, 0, 0, 0};// Start of queue for wait'ing threads
 
 muto NO_COPY *sync_proc_subproc = NULL;	// Control access to subproc stuff
@@ -406,8 +407,6 @@ proc_terminate (void)
       for (i = 0; i < nprocs; i++)
 	{
 	  procs[i]->ppid = 1;
-	  if (!proc_exists (procs[i]))
-	    procs[i]->process_state = PID_EXITED;	/* CGF FIXME - still needed? */
 	  if (procs[i].wait_thread)
 	    {
 	      // CloseHandle (procs[i].rd_proc_pipe);
@@ -860,7 +859,15 @@ remove_proc (int ci)
 	ForceCloseHandle1 (procs[ci].hProcess, childhProc);
     }
   if (ci < --nprocs)
-    procs[ci] = procs[nprocs];
+    {
+      /* Wait for proc_waiter thread to make a copy of this element before
+	 moving it or it may become confused.  The chances are very high that
+	 the proc_waiter thread has already done this by the time we
+	 get here.  */
+      while (!procs[nprocs].waiter_ready)
+	low_priority_sleep (0);
+      procs[ci] = procs[nprocs];
+    }
   return 0;
 }