* external.cc (fillout_pinfo): Remove nonsensical loop.

* fork.cc (frok::parent): When initializing pinfo for child new PID_NEW flag +
actual defined constant rather than raw number.  Don't set start_time here.
* pinfo.cc (pinfo::thisproc): Use PID_NEW when initializing pinfo.  Avoid
checking h for NULL multiple times.  Don't set start_time here.
(pinfo_init): Aways set ppid last.  Tweak strace output.
(pinfo::init): Handle new PID_NEW flag.  Wait for shared memory to contain
useful information.  Set start_time if PID_NEW.
(_onreturn:h): Define as HANDLE rather than HANDLE *.
(_onreturn::~onreturn): Accommodate h definition change.
(_onreturn::no_close_handle): Rename from no_close_p_handle.  Take a pinfo arg
and set hProcess to h before zeroing.
(winpids::add): Don't open a handle to our own process.  Change logic
associated with when a handle gets closed.  Accommodate no_close_handle
changes.
(winpids::enum_processes): Simplify process enumeration loop.
(winpids::set): Eliminate ill-considered malloc locking.
* sigproc.cc (proc_subproc): Always set ppid last.

6 files changed, 83 insertions, 62 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 79b4e3cba..8c973f901 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,27 @@
 2013-12-17  Christopher Faylor  <me.cygwin2013@cgf.cx>
 
+	* external.cc (fillout_pinfo): Remove nonsensical loop.
+	* fork.cc (frok::parent): When initializing pinfo for child new PID_NEW
+	flag + actual defined constant rather than raw number.  Don't set
+	start_time here.
+	* pinfo.cc (pinfo::thisproc): Use PID_NEW when initializing pinfo.
+	Avoid checking h for NULL multiple times.  Don't set start_time here.
+	(pinfo_init): Aways set ppid last.  Tweak strace output.
+	(pinfo::init): Handle new PID_NEW flag.  Wait for shared memory to
+	contain useful information.  Set start_time if PID_NEW.
+	(_onreturn:h): Define as HANDLE rather than HANDLE *.
+	(_onreturn::~onreturn): Accommodate h definition change.
+	(_onreturn::no_close_handle): Rename from no_close_p_handle.  Take a
+	pinfo arg and set hProcess to h before zeroing.
+	(winpids::add): Don't open a handle to our own process.  Change logic
+	associated with when a handle gets closed.  Accommodate no_close_handle
+	changes.
+	(winpids::enum_processes): Simplify process enumeration loop.
+	(winpids::set): Eliminate ill-considered malloc locking.
+	* sigproc.cc (proc_subproc): Always set ppid last.
+
+2013-12-17  Christopher Faylor  <me.cygwin2013@cgf.cx>
+
 	* sigproc.cc (sig_send): Set PIPE_NOWAIT for pipes which are not us.
 
 2013-12-17  Christopher Faylor  <me.cygwin2013@cgf.cx>
diff --git a/winsup/cygwin/external.cc b/winsup/cygwin/external.cc
index 0027c5c00..4dc87bf80 100644
--- a/winsup/cygwin/external.cc
+++ b/winsup/cygwin/external.cc
@@ -76,13 +76,6 @@ fillout_pinfo (pid_t pid, int winpid)
 	}
       else if (nextpid || p->pid == pid || (winpid && thispid == (DWORD) pid))
 	{
-	  /* It is possible that this pinfo is not completely set up yet.  Wait
-	     a while if so.  */
-	  for (int i = 0; i < 2000; i++)
-	    if (p->start_time)
-	      break;
-	    else
-	      Sleep (1);
 	  ep.ctty = (p->ctty < 0 || iscons_dev (p->ctty)) ? p->ctty : device::minor (p->ctty);
 	  ep.pid = p->pid;
 	  ep.ppid = p->ppid;
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 58b109fa9..faccb482c 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -344,7 +344,6 @@ frok::parent (volatile char * volatile stack_here)
   syscall_printf ("CreateProcessW (%W, %W, 0, 0, 1, %y, 0, 0, %p, %p)",
 		  myself->progname, myself->progname, c_flags, &si, &pi);
   bool locked = __malloc_lock ();
-  time_t start_time = time (NULL);
 
   /* Remove impersonation */
   cygheap->user.deimpersonate ();
@@ -412,7 +411,7 @@ frok::parent (volatile char * volatile stack_here)
   fix_impersonation = false;
 
   child_pid = cygwin_pid (pi.dwProcessId);
-  child.init (child_pid, 1, NULL);
+  child.init (child_pid, PID_IN_USE | PID_NEW, NULL);
 
   if (!child)
     {
@@ -421,7 +420,6 @@ frok::parent (volatile char * volatile stack_here)
       goto cleanup;
     }
 
-  child->start_time = start_time; /* Register child's starting time. */
   child->nice = myself->nice;
 
   /* Initialize things that are done later in dll_crt0_1 that aren't done
diff --git a/winsup/cygwin/include/sys/cygwin.h b/winsup/cygwin/include/sys/cygwin.h
index ca2f92996..705aa7f64 100644
--- a/winsup/cygwin/include/sys/cygwin.h
+++ b/winsup/cygwin/include/sys/cygwin.h
@@ -220,7 +220,7 @@ enum
   PID_MYSELF	       = 0x00200, /* Flag that pid is me. */
   PID_NOCLDSTOP	       = 0x00400, /* Set if no SIGCHLD signal on stop. */
   PID_INITIALIZING     = 0x00800, /* Set until ready to receive signals. */
-  PID_UNUSED1	       = 0x01000, /* Available. */
+  PID_NEW	       = 0x01000, /* Available. */
   PID_ALLPIDS	       = 0x02000, /* used by pinfo scanner */
   PID_EXECED	       = 0x04000, /* redirect to original pid info block */
   PID_NOREDIR	       = 0x08000, /* don't redirect if execed */
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 51d01322c..8f9986fc2 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -56,16 +56,21 @@ pinfo::thisproc (HANDLE h)
 {
   procinfo = NULL;
 
+  DWORD flags = PID_IN_USE | PID_ACTIVE;
   if (!h)
-    cygheap->pid = cygwin_pid (myself_initial.pid);
+    {
+      h = INVALID_HANDLE_VALUE;
+      cygheap->pid = cygwin_pid (myself_initial.pid);
+      flags |= PID_NEW;
+    }
 
-  init (cygheap->pid, PID_IN_USE, h ?: INVALID_HANDLE_VALUE);
+  init (cygheap->pid, flags, h);
   procinfo->process_state |= PID_IN_USE;
   procinfo->dwProcessId = myself_initial.pid;
   procinfo->sendsig = myself_initial.sendsig;
   wcscpy (procinfo->progname, myself_initial.progname);
   debug_printf ("myself dwProcessId %u", procinfo->dwProcessId);
-  if (h)
+  if (h != INVALID_HANDLE_VALUE)
     {
       /* here if execed */
       static pinfo NO_COPY myself_identity;
@@ -73,9 +78,6 @@ pinfo::thisproc (HANDLE h)
       procinfo->exec_sendsig = NULL;
       procinfo->exec_dwProcessId = 0;
     }
-  else if (!child_proc_info)	/* child_proc_info is only set when this process
-				   was started by another cygwin process */
-    procinfo->start_time = time (NULL); /* Register our starting time. */
 }
 
 /* Initialize the process table entry for the current task.
@@ -94,20 +96,20 @@ pinfo_init (char **envp, int envc)
       /* Invent our own pid.  */
 
       myself.thisproc (NULL);
-      myself->ppid = 1;
       myself->pgid = myself->sid = myself->pid;
       myself->ctty = -1;
       myself->uid = ILLEGAL_UID;
       myself->gid = UNKNOWN_GID;
       environ_init (NULL, 0);	/* call after myself has been set up */
       myself->nice = winprio_to_nice (GetPriorityClass (GetCurrentProcess ()));
+      myself->ppid = 1;		/* always set last */
       debug_printf ("Set nice to %d", myself->nice);
     }
 
   myself->process_state |= PID_ACTIVE;
   myself->process_state &= ~(PID_INITIALIZING | PID_EXITED | PID_REAPED);
   myself.preserve ();
-  debug_printf ("pid %d, pgid %d", myself->pid, myself->pgid);
+  debug_printf ("pid %d, pgid %d, process_state %y", myself->pid, myself->pgid, myself->process_state);
 }
 
 DWORD
@@ -314,11 +316,15 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
       /* Detect situation where a transitional memory block is being retrieved.
 	 If the block has been allocated with PINFO_REDIR_SIZE but not yet
 	 updated with a PID_EXECED state then we'll retry.  */
-      MEMORY_BASIC_INFORMATION mbi;
-      if (!created && procinfo->exists ()
-	  && VirtualQuery (procinfo, &mbi, sizeof (mbi))
-	  && mbi.RegionSize < sizeof (_pinfo))
-	goto loop;
+      if (!created && !(flag & PID_NEW))
+	{
+	  MEMORY_BASIC_INFORMATION mbi;
+	  for (int i = 0; i < 1000 && !procinfo->ppid; i++)
+	    Sleep (0);
+	  if (procinfo->exists () && VirtualQuery (procinfo, &mbi, sizeof (mbi))
+	      && mbi.RegionSize < sizeof (_pinfo))
+	    goto loop;
+	}
 
       if (!created && createit && (procinfo->process_state & PID_REAPED))
 	{
@@ -360,6 +366,8 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
 	  goto loop;
 	}
 
+      if (flag & PID_NEW)
+	procinfo->start_time = time (NULL);
       if (!created)
 	/* nothing */;
       else if (!(flag & PID_EXECED))
@@ -1112,19 +1120,23 @@ cygwin_winpid_to_pid (int winpid)
 #define size_pinfolist(i) (sizeof (pinfolist[0]) * ((i) + 1))
 class _onreturn
 {
-  HANDLE *h;
+  HANDLE h;
 public:
   ~_onreturn ()
   {
-    if (h && *h)
+    if (h)
       {
-	CloseHandle (*h);
-	*h = NULL;
-	h = NULL;
+	CloseHandle (h);
       }
   }
-  void no_close_p_handle () {h = NULL;}
-  _onreturn (HANDLE& _h): h (&_h) {}
+  void no_close_handle (pinfo& p)
+  {
+    p.hProcess = h;
+    h = NULL;
+  }
+  _onreturn (): h (NULL) {}
+  void operator = (HANDLE h0) {h = h0;}
+  operator HANDLE () const {return h;}
 };
 
 inline void
@@ -1139,23 +1151,28 @@ winpids::add (DWORD& nelem, bool winpid, DWORD pid)
       pinfolist = (pinfo *) realloc (pinfolist, size_pinfolist (npidlist + 1));
     }
 
+  _onreturn onreturn;
   pinfo& p = pinfolist[nelem];
   memset (&p, 0, sizeof (p));
 
-  /* Open a process to prevent a subsequent exit from invalidating the
-     shared memory region. */
-  p.hProcess = OpenProcess (PROCESS_QUERY_INFORMATION, false, pid);
-  _onreturn onreturn (p.hProcess);
-
-  /* If we couldn't open the process then we don't have rights to it and should
-     make a copy of the shared memory area if it exists (it may not).  */
   bool perform_copy;
-  if (!p.hProcess)
-    perform_copy = true;
+  if (cygpid == myself->pid)
+    {
+      p = myself;
+      perform_copy = false;
+    }
   else
-    perform_copy = make_copy;
+    {
+      /* Open a process to prevent a subsequent exit from invalidating the
+	 shared memory region. */
+      onreturn = OpenProcess (PROCESS_QUERY_INFORMATION, false, pid);
 
-  p.init (cygpid, PID_NOREDIR | pinfo_access, NULL);
+      /* If we couldn't open the process then we don't have rights to it and should
+	 make a copy of the shared memory area when it exists (it may not).  */
+      perform_copy = onreturn ? make_copy : true;
+
+      p.init (cygpid, PID_NOREDIR | pinfo_access, NULL);
+    }
 
   /* If we're just looking for winpids then don't do any special cygwin "stuff* */
   if (winpid)
@@ -1170,7 +1187,7 @@ winpids::add (DWORD& nelem, bool winpid, DWORD pid)
       p.init (cygpid, PID_NOREDIR, NULL);
       if (!p)
 	return;
-      }
+    }
 
   /* Scan list of previously recorded pids to make sure that this pid hasn't
      shown up before.  This can happen when a process execs. */
@@ -1212,12 +1229,12 @@ out:
 	/* handle specially.  Close the handle but (eventually) don't
 	   deallocate procinfo in release call */;
       else if (!perform_copy)
-	onreturn.no_close_p_handle ();	/* Don't close the handle until release */
+	onreturn.no_close_handle (p);	/* Don't close the handle until release */
       else
 	{
 	  _pinfo *pnew = (_pinfo *) malloc (sizeof (*p.procinfo));
 	  if (!pnew)
-	    onreturn.no_close_p_handle ();
+	    onreturn.no_close_handle (p);
 	  else
 	    {
 	      *pnew = *p.procinfo;
@@ -1246,23 +1263,17 @@ winpids::enum_processes (bool winpid)
   HANDLE dir = get_shared_parent_dir ();
   BOOLEAN restart = TRUE;
 
-  do
+  while (NT_SUCCESS (NtQueryDirectoryObject (dir, &f, sizeof f, TRUE, restart,
+					     &context, NULL)))
     {
-      status = NtQueryDirectoryObject (dir, &f, sizeof f, TRUE, restart,
-				       &context, NULL);
-      if (NT_SUCCESS (status))
+      restart = FALSE;
+      f.dbi.ObjectName.Buffer[f.dbi.ObjectName.Length / sizeof (WCHAR)] = L'\0';
+      if (wcsncmp (f.dbi.ObjectName.Buffer, L"cygpid.", 7) == 0)
 	{
-	  restart = FALSE;
-	  f.dbi.ObjectName.Buffer[f.dbi.ObjectName.Length / sizeof (WCHAR)]
-	    = L'\0';
-	  if (wcsncmp (f.dbi.ObjectName.Buffer, L"cygpid.", 7) == 0)
-	    {
-	      DWORD pid = wcstoul (f.dbi.ObjectName.Buffer + 7, NULL, 10);
-	      add (nelem, false, pid);
-	    }
+	  DWORD pid = wcstoul (f.dbi.ObjectName.Buffer + 7, NULL, 10);
+	  add (nelem, false, pid);
 	}
     }
-  while (NT_SUCCESS (status));
   cygwin_pid_nelem = nelem;
 
   if (winpid)
@@ -1332,18 +1343,15 @@ winpids::enum_processes (bool winpid)
 	  px = (PSYSTEM_PROCESS_INFORMATION) ((char *) px + px->NextEntryOffset);
 	}
     }
-
   return nelem;
 }
 
 void
 winpids::set (bool winpid)
 {
-  __malloc_lock ();
   npids = enum_processes (winpid);
   if (pidlist)
     pidlist[npids] = 0;
-  __malloc_unlock ();
 }
 
 DWORD
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index c65e7bb37..2dfec5a0b 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -207,7 +207,6 @@ proc_subproc (DWORD what, uintptr_t val)
     case PROC_DETACHED_CHILD:
       if (vchild != myself)
 	{
-	  vchild->ppid = what == PROC_DETACHED_CHILD ? 1 : myself->pid;
 	  vchild->uid = myself->uid;
 	  vchild->gid = myself->gid;
 	  vchild->pgid = myself->pgid;
@@ -215,6 +214,7 @@ proc_subproc (DWORD what, uintptr_t val)
 	  vchild->ctty = myself->ctty;
 	  vchild->cygstarted = true;
 	  vchild->process_state |= PID_INITIALIZING;
+	  vchild->ppid = what == PROC_DETACHED_CHILD ? 1 : myself->pid;	/* always set last */
 	}
       if (what == PROC_DETACHED_CHILD)
 	break;