Cygwin: winpids: Fix getting process multiple times

Switching to Cywin-only PIDs introduced a new problem when collecting
Cygwin processes for `ps -W': A process can show up multiple times
again, if the Cygwin procinfo has been opened for a just execing
process.  The execed process then shows up twice, once as Cygwin
process, but with the wrong Windows PID of the execing process,
once as Windows-only process.

The mechanism used to exclude these stray processes didn't work with
the new Cygwin pid handling anymore.  To fix this

* check if the incoming Windows PID is the same as the PID in the
  procinfo.  If not, we have the PID of the execing process while
  procinfo was already changed,
* always check if the process has already been handled, not only
  for processes we got a procinfo for,
* simplify adding pid to pidlist since pid is now always correct.

While at it, fix comments and comment formatting.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
This commit is contained in:
Corinna Vinschen 2019-03-27 13:53:32 +01:00
parent e8b23909e4
commit d1be0a59d4
1 changed files with 22 additions and 24 deletions

View File

@ -1437,11 +1437,15 @@ winpids::add (DWORD& nelem, bool winpid, DWORD pid)
shared memory region. */ shared memory region. */
onreturn = OpenProcess (PROCESS_QUERY_LIMITED_INFORMATION, false, pid); onreturn = OpenProcess (PROCESS_QUERY_LIMITED_INFORMATION, false, pid);
/* If we couldn't open the process then we don't have rights to it and should /* If we couldn't open the process then we don't have rights to it
make a copy of the shared memory area when it exists (it may not). */ and should make a copy of the shared memory area when it exists
(it may not). */
perform_copy = onreturn ? make_copy : true; perform_copy = onreturn ? make_copy : true;
p.init (cygpid, PID_PROCINFO | pinfo_access, NULL); p.init (cygpid, PID_PROCINFO | pinfo_access, NULL);
/* Did we catch the process during exec? Try to fix. */
if (p && p->dwProcessId != pid)
pid = p->dwProcessId;
} }
/* If we're just looking for winpids then don't do any special cygwin "stuff* */ /* If we're just looking for winpids then don't do any special cygwin "stuff* */
@ -1462,40 +1466,33 @@ winpids::add (DWORD& nelem, bool winpid, DWORD pid)
return; return;
} }
out:
/* Scan list of previously recorded pids to make sure that this pid hasn't /* Scan list of previously recorded pids to make sure that this pid hasn't
shown up before. This can happen when a process execs. */ shown up before. This can happen when a process execs. */
for (unsigned i = 0; i < nelem; i++) for (unsigned i = 0; i < nelem; i++)
if (pinfolist[i]->pid == p->pid) if (pidlist[i] == pid)
{ {
if ((_pinfo *) p != (_pinfo *) myself) if (p && (_pinfo *) p != (_pinfo *) myself)
p.release (); p.release ();
return; return;
} }
/* If p is "false" then, eventually any opened process handle will be closed
out: and the function will exit without adding anything to the pid list.
/* Exit here.
If p is "false" then, eventually any opened process handle will be closed and
the function will exit without adding anything to the pid list.
If p is "true" then we've discovered a cygwin process. If p is "true" then we've discovered a cygwin process.
Handle "myself" differently. Don't copy it and close/zero the handle we Handle "myself" differently. Don't copy it and close/zero the handle we
just opened to it. just opened to it. If not performing a copy, then keep the process handle
If not performing a copy, then keep the process handle open for the duration open for the duration of the life of the procinfo region to potential
of the life of the procinfo region to potential races when a new process uses races when a new process uses this pid. Otherwise, malloc some memory
this pid. for a copy of the shared memory.
Otherwise, malloc some memory for a copy of the shared memory.
If the malloc failed, then "oh well". Just keep the shared memory around If malloc failed, then "oh well". Just keep the shared memory around
and eventually close the handle when the winpids goes out of scope. and eventually close the handle when the winpids goes out of scope.
If malloc succeeds, copy the procinfo we just grabbed into the new region, If malloc succeeds, copy the procinfo we just grabbed into the new region,
release the shared memory and allow the handle to be closed when this release the shared memory and allow the handle to be closed when this
function returns. function returns. */
Oh, and add the pid to the list and bump the number of elements. */
if (p) if (p)
{ {
if (p == (_pinfo *) myself) if (p == (_pinfo *) myself)
@ -1519,8 +1516,9 @@ out:
} }
} }
} }
/* Add pid to the list and bump the number of elements. */
if (p || winpid) if (p || winpid)
pidlist[nelem++] = !p ? pid : p->dwProcessId; pidlist[nelem++] = pid;
} }
DWORD DWORD
@ -1545,9 +1543,9 @@ winpids::enum_processes (bool winpid)
f.dbi.ObjectName.Buffer[f.dbi.ObjectName.Length / sizeof (WCHAR)] = L'\0'; f.dbi.ObjectName.Buffer[f.dbi.ObjectName.Length / sizeof (WCHAR)] = L'\0';
if (wcsncmp (f.dbi.ObjectName.Buffer, L"winpid.", 7) == 0) if (wcsncmp (f.dbi.ObjectName.Buffer, L"winpid.", 7) == 0)
{ {
DWORD pid = wcstoul (f.dbi.ObjectName.Buffer + 7, NULL, 10); DWORD pid = wcstoul (f.dbi.ObjectName.Buffer + 7, NULL, 10);
add (nelem, false, pid); add (nelem, false, pid);
} }
} }
} }
else else