Get rid of fallback to process ids enumeration in ProcessManager.IsProcessRunning (Windows)

[epeshk]

Currently, when process Handle can't be opened for any reason (local machine case), ProcessManager.IsProcessRunning enumerates all processes in the system to ensure that no process with given Id exists. This enumeration implemented via EnumProcesses call which is very slow (~10ms) compared to good case when handle is successfully opened

I want to suggest some improvements to (mostly) get rid of this fallback to achieve better performance.

Main suggestion is assume that process doesn't exists when OpenProcess failed not due to lack of permissions (ERROR_ACCESS_DENIED).

But there are some special cases should be considered.

Handle to process couldn't be opened:

• for Idle processes (id=0) (ERROR_INVALID_PARAMETER)
• for protected (e.g. csrss.exe) processes (ERROR_ACCESS_DENIED)
• any other cases when our process is limited? maybe there is something special on Nano Server or old/future versions of Windows (who knows?)

In these cases we could (1 OR 2 OR 3):

1. fall back to slow enumeration (looks like the safest way, cases when enumeration does happen don't seem to be common)
2. optimistically assume that process is still running (allows to fully remove enumeration for local machine, but who knows if any other corner cases exists)
3. other suggestions?

System.Diagnostics.ProcessManager.OpenProcess still assumes ERROR_ACCESS_DENIED means still running process. So second way doesn't look completely wrong.

What do you think, which way should be used? In this PR, I've implemented the first way, but it's not a problem to rewrite with second way if these optimistic assumptions are reasonable.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Currently, when process Handle can't be opened for any reason (local machine case), ProcessManager.IsProcessRunning enumerates all processes in the system to ensure that no process with given Id exists. This enumeration implemented via EnumProcesses call which is very slow (~10ms) compared to good case when process exists and handle is successfully opened

I want to suggest some improvements to (mostly) get rid of this fallback to achieve better performance.

Main suggestion is assume that process doesn't exists when OpenProcess failed not due to lack of permissions (ERROR_ACCESS_DENIED).

But there are some special cases should be considered.

Handle to process couldn't be opened:

• for Idle processes (id=0) (ERROR_INVALID_PARAMETER)
• for protected (e.g. csrss.exe) processes (ERROR_ACCESS_DENIED)
• any other cases when our process is limited? maybe there is something special on Nano Server or old/future versions of Windows (who knows?)

In these cases we could (1 OR 2 OR 3):

1. fall back to slow enumeration (looks like the safest way, cases when enumeration does happen don't seem to be common)
2. optimistically assume that process is still running (allows to fully remove enumeration for local machine, but who knows if any other corner cases exists)
3. other suggestions?

System.Diagnostics.ProcessManager.OpenProcess still assumes ERROR_ACCESS_DENIED means still running process. So second way doesn't look completely wrong.

What do you think, which way should be used? In this PR, I've implemented the first way, but it's not a problem to rewrite with second way if these optimistic assumptions are reasonable.

Author:	epeshk
Assignees:	-
Labels:	area-System.Diagnostics.Process, community-contribution
Milestone:	-

[adamsitnik]

Hi @epeshk

Big thanks for your contribution and providing a very descriptive explanation. Please take a look at my comment and let me know what do you think about it.

Could you please say a little bit more about the scenario in which you are using this method and the optimization would help? Are you frequently calling Process.GetProcessById for pid that does not exist? If so, may I ask why? (I am trying to get a better understanding)

[adamsitnik]

The OpenProcess method can fail for multiple reasons. I am not 100% sure that all errors except ERROR_ACCESS_DENIED mean that the process does not exist. However, I think that it's safe to assume that when the method fails with ERROR_INVALID_PARAMETER the process does not exist and we can safely return false (I've tested it locally).

Suggested change

	if (error != Interop.Errors.ERROR_ACCESS_DENIED)
	return false;
	if (error == Interop.Errors.ERROR_INVALID_PARAMETER)
	{
	Debug.Assert(processId != 0, "OpenProcess fails with ERROR_INVALID_PARAMETER for Idle Process");
	return false;
	}

[adamsitnik]

LGTM, thank you @epeshk !