[API Proposal]: Process.TryGetProcessById()

[mqudsi]

Background and motivation

The existing api for retrieving a process by its process id (Process.GetProcessById()) does not provide any exception-safe method of querying a process, as it throws an ArgumentException if the specified pid cannot be found and there is no means of atomically guaranteeing that a once-valid process id will remain valid by the time the Process.GetProcessById() call is being serviced.

This is true regardless of where the pid was obtained from, even if the pid were obtained from a previous call to Process.GetProcessById() due to the inherently concurrent execution model associated with multiprocessing.

If it's any sort of additional incentive, the exception thrown by Process.GetProcessById() when a valid-but-no-longer-extant pid is provided is ArgumentException, which is normally not an exception that a calling library/application should be catching. Moreover, the reuse of ArgumentException to indicate both "process not found" and "invalid machine name" (for the two-parameter override) adds to the confusion of using this api, as one case is intended to be caught and the other, generally speaking, is not.

API Proposal

namespace System.Diagnostics;

public class Process
{
    public static System.Diagnostics.Process? TryGetProcessById(int pid);
    public static System.Diagnostics.Process? TryGetProcessById(int pid, string machineName);
}

API Usage

// Use shared memory IPC to get PID of other instance
var pid = 42;

using var process = Process.TryGetProcessById(pid);
if (process is not null)
{
    process.WaitForExit();
}

Alternative Designs

I know the preferred approach for TryFoo() methods is to return a boolean indicating success or failure and use an out param to provide the output on success, but in this case I eschewed that approach in favor of returning a nullable Process? directly to make sure it remains easy to remember to always use using with the disposable Process object.

Otherwise, the API might look as follows:

class Process
{
    public static bool TryGetProcessById(int pid, out System.Diagonstics.Process process);
    public static bool TryGetProcessById(int pid, string machineName, out System.Diagnostics.Process process);
}

but that would be prone to misuse.

Risks

Some might be concerned about the proliferation of TryFoo() methods, though working around the unfortunate legacy use of ArgumentException here and the inherently fallible nature of obtaining a process (that may have already exited) by its id really lend themselves to recommending the addition of an exception-free alternative to the current Process.GetProcessById() method.

[dotnet-policy-service]

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

[daniel-white]

The Try... pattern usually has an out parameter and return a Boolean. I would expect this api to work the same way

[stephentoub]

Can you elaborate on the scenario where this is needed? You're on some path where the overhead of the exception is prohibitive?

[tannergooding]

I don't think that Try* APIs should be restricted to just cases where exception overhead is prohibitive or performance sensitive areas.

Exceptions are massively expensive, they can be error prone to handle, may not actually cover what the user wanted to cover over time, and it is generally considered best practice to not do control flow based on exceptions. Forcing users to handle them in scenarios that are known to be prone to arbitrary failure (looking up process IDs, opening or creating files, etc) just because the operation itself already has a larger bit of overhead doesn't make sense to me and seems to be encouraging practices that are widely touted as being bad or problematic.

The general try { } catch (...) { } pattern is also tricky for many different reasons and can be difficult to integrate into general control flow paths. A user can always define their own helper TryGetProcessId path that itself does the try/catch and converts the exception to bool, but that's a lot more complex and really inverts the more normal expectation (which is simply exposing it ourselves such that GetProcessId calls TryGetProcessId and throw if it returns false; and therefore achieves the same thing but in a stable/documented way that also gives it to the user as cheaply as possible and without any significant overhead required on our part).

[stephentoub]

I don't think that Try* APIs should be restricted to just cases where exception overhead is prohibitive or performance sensitive areas.

That's an extremely slippery slope. .NET uses exceptions for these cases. From my perspective, that argument effectively leads to saying that any method which could throw (which is many) could have a Try method just to represent failure differently, and that's not a path we're going to go down.

[danmoseley]

I've seen this discussion a few times on API proposals in the past and generally they returned to the consensus that .NET for good or ill chose the exception model for error handling and the Try pattern is only for the highest performance cases where errors are inevitable e.g. parsing.

If we use the Try pattern more broadly, what criteria do you believe we should use to decide whether it is worthwhile to add the duplicate API?

[daniel-white]

For this theres a cost even in determining whether the process exists or not. Looking at the implementation its using kill on *ix, so would the cost for an exception be still higher? perhaps a helper method to see if the process is still alive then attempt to use GetProcessById after might be better? then again theres a race. this could be easily implemented by the caller.

[tannergooding]

That's an extremely slippery slope. .NET uses exceptions for these cases. From my perspective, that argument effectively leads to saying that any method which could throw (which is many) could have a Try method just to represent failure differently, and that's not a path we're going to go down.

I disagree. This isn't a statement that try can/should be provided for "any method" which could throw, it's specifically keeping the spirit of Try* APIs as detailed in the FDG

CONSIDER the Try Pattern for members that might throw exceptions in common scenarios to avoid performance problems related to exceptions.

The only difference is that it's saying that "to avoid performance problems related to exceptions" should be extended to basically include something along the lines of "or where the user performing their own pre-validation is impossible".

So something like TimeSpan.FromHours should not have some TimeSpan.TryFromHours, this doesn't make sense. A user can trivially validate it themselves before hand and has many avenues for avoiding the exception.

However, something like Process.GetProcessId or File.Delete are cases where there is literally nothing a user can do except for catch the exception that is currently being thrown. This is because they are explicitly interacting with systems outside the control of the process which may be getting changed concurrently. It's the same reason why if (File.Exists(...) { File.Delete(...); } is problematic and why issues like #27217 have been opened.

There is a very clear and distinguished line we can draw that keeps this from being a slippery slope. Which provides clear and concise benefit to users that are using these APIs while maintaining the spirit of the guideline and how users generally expect Try APIs to work.

[tannergooding]

We are ourselves functionally using system level "try apis" for most of the functions in this category already and simply converting them to exceptions 100% of the time.

So it's really just an ask to expose the functionality the system already provides because the system already understands that it needs to exist that way.

[stephentoub]

The only difference is that it's saying that "to avoid performance problems related to exceptions" should be extended to basically include something along the lines of "or where the user performing their own pre-validation is impossible".

And I believe that extension includes the vast majority of exceptions we end up throwing.

it's specifically keeping the spirit of Try* APIs as detailed in the FDG

I disagree. That "or" is a very large leap from the original spirit.

[tannergooding]

And I believe that extension includes the vast majority of exceptions we end up throwing.

Do you have an example of what you think it would include?

The things people have historically asked for these APIs around are explicitly APIs like file handling or things involving processes and that other languages and the system APIs themselves provide "try" like APIs for.

It feels like a disservice to .NET users to keep pushing off exposing such functionality ourselves due to the potential for it to be a slipper slope or belief that the performance characteristics don't matter.

[stephentoub]

Do you have an example of what you think it would include?

As one example of many, Stream.Read; that can throw exceptions for any number of reasons where "the user performing their own pre-validation is impossible".

[tannergooding]

Stream is an example of IO and would qualify as something a try API should really exist for IMO. It's functionally the same concept as doing other types of IO, a case where such try APIs have been explicitly requested in the past, and where other languages or the system level APIs themselves expose the ability to do the functionality in a non-faulting form.

IOException in general is a great example of where it is commonly seen as part of a catch (IOException) { } block that only exists to ignore the exception and allow the user or code to refresh and try again, ideally with a different input and so really fits in with the type of scenario that Try* APIs are explicitly good for.

[tannergooding]

To me, and I think to a pretty decent part of the .NET community, Try* APIs should in general exist for any kind of exception that cannot be trivially handled by the developer and for which handling the point of failure is an expected thing in real world apps.

Things like StackOverflowException or OutOfMemoryException are exceptions the developer can't handle, but are incredibly rare and essentially "unexpected". They additionally represent "catastrophic failure", so there is nothing they can reasonably do even if they could catch it.

Things like ArgumentException are exceptions that the developer can handle and which are also supposed to be rare and essentially "unexpected" in production code. When they exist, they typically represent bugs in the user code, so the fix is to adjust your code to not pass in the bad state.

Things like FormatException or IOException are then cases where they are common and expected, largely because they are used with inputs or data that exists externally to the process and which cannot be trivially handled. It is generally expected for an application to need to handle such things, recover, and continue, and so they are very good examples of places where Try* APIs should exist to give developers not only contextual awareness that there is some common error state they may want to handle, but to allow them to easily integrate that handling into their control flow.

[JimMore]

If a method can fail in normal use, it qualifies for a Try* API.
Yes, Stream.Read can fail, but that's an implementation detail that doesn't apply to all streams.
In the case of Process.GetProcessById(), the only implementation is prone to error in normal use.

[stephentoub]

Try* APIs should in general exist for any kind of exception that cannot be trivially handled by the developer and for which handling the point of failure is an expected thing in real world apps.

And this is broadly arguing against .NET's general exception model and philosophy. It's been litigated in the past. I'm not going to relitigate it here.

[mqudsi]

Things like ArgumentException are exceptions that the developer can handle and which are also supposed to be rare and essentially "unexpected" in production code. When they exist, they typically represent bugs in the user code, so the fix is to adjust your code to not pass in the bad state.

And this is precisely not the case for Process.GetProcessById(), where it's perfectly normal for a process to have exited in between the time you obtained its pid and when you queried for it, isn't a bug in most cases, and can't be validated or checked for atomically when using this api, but it throws ArgumentException anyway.

[mqudsi]

@daniel-white

The Try... pattern usually has an out parameter and return a Boolean. I would expect this api to work the same way

Yes, I pointed out the reason for diverging from that in the first post under "Alternative Designs". But maybe I missed a way that would work.

[danmoseley]

Requiring to the original proposal. It's certainly true that it is not possible for the developer to avoid potential exceptions. But I'm not clear what the motivation is in this particular case to avoid an exception. Are you seeing a performance problem from so many exceptions? Or it's about writing more compact code?

[danmoseley]

And for the general point, I don't have the FDG in front of me, but my recollection is this falls out of the scope they define for the Try pattern. Assuming that's the case (@bartonjs) then the first thing to do if you want this API is to get the FDG changed, so we don't debate this case by case.

[jkotas]

I'm not clear what the motivation is in this particular case to avoid an exception.

This API proposal was motivated by a discussion about merits of catching ArgumentExceptions in unrelated issue.

[mqudsi]

@danmoseley I apologize, I neglected to answer @stephentoub's original question.

This particular requests was not motivated by performance considerations; it came about when I was searching through my code for places where I catch ArgumentException.

I'm not arguing for making exception-free versions of all core apis, but was bothered by the fact that attempting to find a process by id when it has already exited is not exceptional (and is rather routine, actually) but still throws. I was also bothered by the specific exception that was thrown, as @jkotas also mentions.

[danmoseley]

Gotcha. With respect to the exception type, it wouldn't be the only case where it arguably wasn't the best choice they made way back. The only option would be to throw a derived type, presumably a new one, and it isn't clear what that could reasonably be.

[danmoseley]

As for not being exceptional - looking for input from @bartonjs on whether that conflicts with FDG. If it does, the discussion (if you want to pursue) should be advocating FDG not changing this specific case.

[bartonjs]

GetProcessById throwing an ArgumentException when there's no process with that ID is not consistent with the guidelines. Or at least with best practice and a Krzysztof personal advice bubble:

KRZYSZTOF CWALINA

The difference between InvalidOperationException and ArgumentException is that ArgumentException does not rely on the state of any other object besides the argument itself to determine whether it needs to be thrown. For example, if client code tries to access a nonexistent resource, InvalidOperationException should be thrown. On the other hand, if client code tries to access a resource using a malformed identifier, ArgumentException should be thrown.

So this should have been something like InvalidOperationException, while a negative PID is presumably actually an ArgumentException.

I know the preferred approach for TryFoo() methods is to return a boolean indicating success or failure and use an out param to provide the output on success, but in this case...

FDG is very clear on this one:

DO “return” the value from a Try method via an out parameter.

and also

DO use the prefix “Try” and Boolean return type for methods implementing the Try pattern.

There is, of course, always room for "maybe we never considered...", but given that this is on the wrong side of two "DOs" (and my gut says we're already had Try methods emitting IDisposable things), it feels unlikely to be the way we choose.

And for the general point...

@stephentoub was already addressing this point. FDG doesn't directly define when not to make them, but what it has to say about when is

CONSIDER the Try Pattern for members that might throw exceptions in common scenarios to avoid performance problems related to exceptions.

The prose also supports doing it for anything with an "inherent race condition" (citing ConcurrentDictionary.TryAdd).

One thing that is potentially difficult for this specific proposal, and for the linked File.TryOpen proposal is

DO pick one kind of reason for why your Try method can return false, and throw an exception for any other failure reason.

For TryGetProcessById, that one reason could be "that PID doesn't exist". If there are permissions associated with the routine then there's a question of should an access denied throw or return false (changing the reason to "I can't open it"). The sniff test is "does the same if-false block handle the failures the same way". Something like top would conceivably render "I can't open this process" as different from "there is no process 187". And then there's the fun of "you wanted me to open a remote process" failing because "I can't talk to the machine", "It timed out", "Access Denied", etc. (though that's easy, just leave off the machine-name-accepting overload)

And, at the end of the day:

DO NOT return error codes.

Exceptions are the primary means of reporting errors in frameworks.

Which basically means that a Try method should only be added for performance reasons when there's a broad (enough) demonstrated need, not just a "but exceptions are expensive!" knee-jerk reaction.

---

All that said, in my personal opinion public static bool TryGetProcessById(int pid, out Process process); seems like a reasonable addition, provided that false means only "there was no process with this ID" (access denied, et al, would still throw), as it addresses the race condition along the lines of "I was told this work is being handled by PID 12345, so I want to open it and call WaitForExit... but it has exited already" (or, if calling WaitForExit makes Steve cry, "to build a Task tracking the out-of-process work")

If the existing API didn't misuse ArgumentException for "your argument doesn't match the runtime state" I'd probably fall back to asking what sort of workload would benefit from this (e.g. for a WaitForExit type thing, the slowness of the exception route is probably still faster than the expected wait time, so there's not (to me) valid a "but... perf!" argument).

I'd also favor adding public class ProcessNotFoundException : ArgumentException { public ProcessNotFoundException(int pid); } as a "here's how to better detect this state". It's kinda icky, though, so I might be more in favor of the Try method.