Cygwin: processes: use dedicated Cygwin PID rather than Windows PID

Using the Windows PID as Cygwin PID has a few drawbacks:

- the PIDs on Windows get reused quickly.  Some POSIX applications choke
  on that, so we need extra code to avoid too quick PID reuse.

- The code to avoid PID reuse keeps parent process handles and
  (depending on a build option) child processes open unnecessarily.

- After an execve, the process has a split personality:  Its Windows PID
  is a new PID, while its Cygwin PID is the PID of the execve caller
  process.  This requires to keep two procinfo shared sections open, the
  second just to redirect process info requests to the first, correct
  one.

This patch changes the way Cygwin PIDs are generated:

- Cygwin PIDs are generated independently of the Windows PID, in a way
  expected by POSIX processes.  The PIDs are created incrementally in
  the range between 2 and 65535, round-robin.

- On startup of the first Cygwin process, choose a semi-random start PID
  for the first process in the lower PID range to make the PIDs slightly
  unpredictable.  This may not be necessary but it seems kind of inviting
  to know that the first Cygwin process always starts with PID 2.

- Every process not only creates the shared procinfo section, but also a
  symlink in the NT namespace, symlinking the Windows PID to the Cygwin
  PID.  This drops the need for the extra procinfo section after execve.

- Don't keep other process handles around unnecessarily.

- Simplify the code creating/opening the shared procinfo section and
  make a clear distinction between interfaces getting a Cygwin PID and
  interfaces getting a Windows PID.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

1 files changed, 128 insertions, 84 deletions

diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 90dfd2b7c..98168c76a 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -58,8 +58,7 @@ pinfo::thisproc (HANDLE h)
   DWORD flags = PID_IN_USE | PID_ACTIVE;
   if (!h)
     {
-      h = INVALID_HANDLE_VALUE;
-      cygheap->pid = cygwin_pid (myself_initial.pid);
+      cygheap->pid = create_cygwin_pid ();
       flags |= PID_NEW;
     }
 
@@ -68,16 +67,10 @@ pinfo::thisproc (HANDLE h)
   procinfo->dwProcessId = myself_initial.pid;
   procinfo->sendsig = myself_initial.sendsig;
   wcscpy (procinfo->progname, myself_initial.progname);
+  create_winpid_symlink (procinfo->pid, procinfo->dwProcessId);
+  procinfo->exec_sendsig = NULL;
+  procinfo->exec_dwProcessId = 0;
   debug_printf ("myself dwProcessId %u", procinfo->dwProcessId);
-  if (h != INVALID_HANDLE_VALUE)
-    {
-      /* here if execed */
-      static pinfo NO_COPY myself_identity;
-      myself_identity.init (cygwin_pid (procinfo->dwProcessId), PID_EXECED, NULL);
-      procinfo->exec_sendsig = NULL;
-      procinfo->exec_dwProcessId = 0;
-      myself_identity->ppid = procinfo->pid;
-    }
 }
 
 /* Initialize the process table entry for the current task.
@@ -109,7 +102,8 @@ pinfo_init (char **envp, int envc)
   myself->process_state |= PID_ACTIVE;
   myself->process_state &= ~(PID_INITIALIZING | PID_EXITED | PID_REAPED);
   myself.preserve ();
-  debug_printf ("pid %d, pgid %d, process_state %y", myself->pid, myself->pgid, myself->process_state);
+  debug_printf ("pid %d, pgid %d, process_state %y",
+		myself->pid, myself->pgid, myself->process_state);
 }
 
 DWORD
@@ -152,7 +146,7 @@ pinfo::status_exit (DWORD x)
 	 reason (but note, the environment *in* CMD is broken and shortened).
 	 This occurs at a point where there's no return to the exec'ing parent
 	 process, so we have to find some way to inform the user what happened.
-	 
+
 	 FIXME: For now, just return with SIGBUS set.  Maybe it's better to add
 	 a lengthy small_printf instead. */
       x = SIGBUS;
@@ -229,6 +223,106 @@ pinfo::exit (DWORD n)
 }
 # undef self
 
+/* Return next free Cygwin PID between 2 and 65535, round-robin.  Each new
+   PID is checked that it doesn't collide with an existing PID.  For that,
+   just check if the "cygpid.PID" section exists. */
+pid_t
+create_cygwin_pid ()
+{
+  pid_t pid = 0;
+  WCHAR sym_name[24];
+  UNICODE_STRING sym_str;
+  OBJECT_ATTRIBUTES attr;
+  HANDLE sym_hdl;
+  NTSTATUS status;
+
+  do
+    {
+      do
+	{
+	  pid = ((uint32_t) InterlockedIncrement (&cygwin_shared->pid_src))
+		% 65536;
+	}
+      while (pid < 2);
+      __small_swprintf (sym_name, L"cygpid.%u", pid);
+      RtlInitUnicodeString (&sym_str, sym_name);
+      InitializeObjectAttributes (&attr, &sym_str, OBJ_CASE_INSENSITIVE,
+				  get_shared_parent_dir (), NULL);
+      /* We just want to know if the section (and thus the process) still
+         exists.  Instead of actually opening the section, try to open
+	 it as symlink.  NtOpenSymbolicLinkObject will always returns an
+	 error:
+	 - STATUS_OBJECT_NAME_NOT_FOUND if the section doesn't exist,
+	   so the slot is free and we can use this pid.
+	 - STATUS_OBJECT_TYPE_MISMATCH if the section exists, so we have
+	   to skip this pid and loop to try the next one.
+	  As side-effect we never have to close the section handle and thus
+	  we don't influence the lifetime of the section. */
+      status = NtOpenSymbolicLinkObject (&sym_hdl, SYMBOLIC_LINK_QUERY, &attr);
+    }
+  while (status == STATUS_OBJECT_TYPE_MISMATCH);
+  return pid;
+}
+
+/* Convert Windows WINPID into Cygwin PID.  Utilize the "winpid.WINPID"
+   symlinks created for each process.  The symlink contains the Cygwin
+   PID as target.  Return 0 if no "winpid.WINPID" symlink exists for
+   this WINPID. */
+pid_t
+cygwin_pid (DWORD dwProcessId)
+{
+  WCHAR sym_name[24];
+  WCHAR pid_name[12];
+  UNICODE_STRING sym_str;
+  UNICODE_STRING pid_str;
+  OBJECT_ATTRIBUTES attr;
+  HANDLE sym_hdl;
+  NTSTATUS status;
+
+  __small_swprintf (sym_name, L"winpid.%u", dwProcessId);
+  RtlInitUnicodeString (&sym_str, sym_name);
+  InitializeObjectAttributes (&attr, &sym_str, OBJ_CASE_INSENSITIVE,
+			      get_shared_parent_dir (), NULL);
+  status = NtOpenSymbolicLinkObject (&sym_hdl, SYMBOLIC_LINK_QUERY, &attr);
+  if (!NT_SUCCESS (status))
+    return 0;
+  RtlInitEmptyUnicodeString (&pid_str, pid_name,
+			     sizeof pid_name - sizeof (WCHAR));
+  status = NtQuerySymbolicLinkObject (sym_hdl, &pid_str, NULL);
+  NtClose (sym_hdl);
+  if (!NT_SUCCESS (status))
+    {
+      system_printf ("NtOpenSymbolicLinkObject: %y, PID %u, ret 0",
+		     status, dwProcessId);
+      return 0;
+    }
+  pid_str.Buffer[pid_str.Length / sizeof (WCHAR)] = L'\0';
+  pid_t ret = (pid_t) wcstoul (pid_str.Buffer, NULL, 10);
+  return ret;
+}
+
+/* Create "winpid.WINPID" symlinks with the Cygwin PID of that process as
+   target.  This is used to find the Cygwin PID for a given Windows WINPID. */
+inline void
+pinfo::create_winpid_symlink (pid_t cygpid, DWORD winpid)
+{
+  WCHAR sym_name[24];
+  WCHAR pid_name[24];
+  UNICODE_STRING sym_str;
+  UNICODE_STRING pid_str;
+  OBJECT_ATTRIBUTES attr;
+
+  __small_swprintf (sym_name, L"winpid.%u",
+		    procinfo->dwProcessId ?: myself_initial.pid);
+  RtlInitUnicodeString (&sym_str, sym_name);
+  __small_swprintf (pid_name, L"%u", procinfo->pid);
+  RtlInitUnicodeString (&pid_str, pid_name);
+  InitializeObjectAttributes (&attr, &sym_str, OBJ_CASE_INSENSITIVE,
+			      get_shared_parent_dir (), NULL);
+  NtCreateSymbolicLinkObject (&winpid_hdl, SYMBOLIC_LINK_ALL_ACCESS,
+			      &attr, &pid_str);
+}
+
 inline void
 pinfo::_pinfo_release ()
 {
@@ -252,20 +346,18 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
 {
   shared_locations shloc;
   h = NULL;
-  if (myself && !(flag & PID_EXECED)
-      && (n == myself->pid || (DWORD) n == myself->dwProcessId))
+  if (myself && n == myself->pid)
     {
       procinfo = myself;
       destroy = 0;
       return;
     }
 
-  int createit = flag & (PID_IN_USE | PID_EXECED);
+  int createit = (flag & PID_IN_USE);
   DWORD access = FILE_MAP_READ
-		 | (flag & (PID_IN_USE | PID_EXECED | PID_MAP_RW)
-		    ? FILE_MAP_WRITE : 0);
+		 | (flag & (PID_IN_USE | PID_MAP_RW) ? FILE_MAP_WRITE : 0);
   if (!h0 || myself.h)
-    shloc = (flag & (PID_IN_USE | PID_EXECED)) ? SH_JUSTCREATE : SH_JUSTOPEN;
+    shloc = (flag & PID_IN_USE) ? SH_JUSTCREATE : SH_JUSTOPEN;
   else
     {
       shloc = SH_MYSELF;
@@ -281,14 +373,8 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
 
   for (int i = 0; i < 20; i++)
     {
-      DWORD mapsize;
-      if (flag & PID_EXECED)
-	mapsize = PINFO_REDIR_SIZE;
-      else
-	mapsize = sizeof (_pinfo);
-
-      procinfo = (_pinfo *) open_shared (L"cygpid", n, h0, mapsize, &shloc,
-					 sec_attribs, access);
+      procinfo = (_pinfo *) open_shared (L"cygpid", n, h0, sizeof (_pinfo),
+					 &shloc, sec_attribs, access);
       if (!h0)
 	{
 	  if (createit)
@@ -311,33 +397,10 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
 
       bool created = shloc != SH_JUSTOPEN;
 
-      /* 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.  */
-      if (!created && !(flag & PID_NEW) && !procinfo->ppid)
-	{
-	  /* Fetching process info for /proc or ps?  just ignore this one. */
-	  if (flag & PID_NOREDIR)
-	    break;
-	  /* FIXME: Do we ever hit this case?  And if so, in what situation? */
-	  system_printf ("This shouldn't happen:\n"
-			 "    me: (%d, %d, %d, %W)\n"
-			 "    pid %d\n"
-			 "    process_state %y\n"
-			 "    cygstarted %d\n"
-			 "    dwProcessId %d\n"
-			 "    name %W",
-			 myself->pid, myself->dwProcessId, myself->cygstarted,
-			 myself->progname,
-			 procinfo->pid, procinfo->process_state,
-			 procinfo->cygstarted, procinfo->dwProcessId,
-			 procinfo->progname);
-	  /* If not populated, wait 2 seconds for procinfo to become populated.
-	     Would like to wait with finer granularity but that is not easily
-	     doable.  */
-	  for (int i = 0; i < 200 && !procinfo->ppid; i++)
-	    Sleep (10);
-	}
+      /* Just fetching info for ps or /proc, don't do anything rash. */
+      if (!created && !(flag & PID_NEW) && !procinfo->ppid
+	  && (flag & PID_PROCINFO))
+	break;
 
       if (!created && createit && (procinfo->process_state & PID_REAPED))
 	{
@@ -346,32 +409,18 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
 				   shared memory */
 	}
 
-      if ((procinfo->process_state & PID_REAPED)
-	  || ((procinfo->process_state & PID_INITIALIZING) && (flag & PID_NOREDIR)
-	      && cygwin_pid (procinfo->dwProcessId) != procinfo->pid))
+      if (procinfo->process_state & PID_REAPED)
 	{
 	  set_errno (ESRCH);
 	  break;
 	}
 
-      if (procinfo->process_state & PID_EXECED)
-	{
-	  pid_t realpid = procinfo->pid;
-	  debug_printf ("execed process windows pid %u, cygwin pid %d", n, realpid);
-	  if (realpid == n)
-	    api_fatal ("retrieval of execed process info for pid %d failed due to recursion.", n);
-
-	  n = realpid;
-	  CloseHandle (h0);
-	  h0 = NULL;
-	  goto loop;
-	}
-
       /* In certain pathological cases, it is possible for the shared memory
 	 region to exist for a while after a process has exited.  This should
 	 only be a brief occurrence, so rather than introduce some kind of
 	 locking mechanism, just loop.  */
-      if (!created && createit && (procinfo->process_state & (PID_EXITED | PID_REAPED)))
+      if (!created && createit
+	  && (procinfo->process_state & (PID_EXITED | PID_REAPED)))
 	{
 	  debug_printf ("looping because pid %d, procinfo->pid %d, "
 			"procinfo->dwProcessid %u has PID_EXITED|PID_REAPED set",
@@ -381,15 +430,8 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
 
       if (flag & PID_NEW)
 	procinfo->start_time = time (NULL);
-      if (!created)
-	/* nothing */;
-      else if (!(flag & PID_EXECED))
+      if (created)
 	procinfo->pid = n;
-      else
-	{
-	  procinfo->process_state |= PID_IN_USE | PID_EXECED;
-	  procinfo->pid = myself->pid;
-	}
 
       h = h0;	/* Success! */
       break;
@@ -529,7 +571,7 @@ _pinfo::set_ctty (fhandler_termios *fh, int flags)
 bool __reg1
 _pinfo::exists ()
 {
-  return process_state && !(process_state & (PID_EXITED | PID_REAPED | PID_EXECED));
+  return process_state && !(process_state & (PID_EXITED | PID_REAPED));
 }
 
 bool
@@ -1279,6 +1321,8 @@ void
 pinfo::release ()
 {
   _pinfo_release ();
+  if (winpid_hdl)
+    NtClose (winpid_hdl);
   HANDLE close_h;
   if (rd_proc_pipe)
     {
@@ -1389,7 +1433,7 @@ winpids::add (DWORD& nelem, bool winpid, DWORD pid)
 	 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);
+      p.init (cygpid, PID_PROCINFO | pinfo_access, NULL);
     }
 
   /* If we're just looking for winpids then don't do any special cygwin "stuff* */
@@ -1403,9 +1447,9 @@ winpids::add (DWORD& nelem, bool winpid, DWORD pid)
      that it isn't a cygwin process. */
   if (!p)
     {
-      if (!pinfo_access)
+      if (!pinfo_access || !cygpid)
 	return;
-      p.init (cygpid, PID_NOREDIR, NULL);
+      p.init (cygpid, PID_PROCINFO, NULL);
       if (!p)
 	return;
     }
@@ -1491,7 +1535,7 @@ winpids::enum_processes (bool winpid)
 	{
 	  restart = FALSE;
 	  f.dbi.ObjectName.Buffer[f.dbi.ObjectName.Length / sizeof (WCHAR)] = L'\0';
-	  if (wcsncmp (f.dbi.ObjectName.Buffer, L"cygpid.", 7) == 0)
+	  if (wcsncmp (f.dbi.ObjectName.Buffer, L"winpid.", 7) == 0)
 	    {
 	    DWORD pid = wcstoul (f.dbi.ObjectName.Buffer + 7, NULL, 10);
 	    add (nelem, false, pid);