Skip to content

System.Diagnostics.Process should not inherit handles by default #19569

Description

@whoisj

When a Windows process starts another process, the parent gets the option of having the child process inherit all of the parent's inheritable handles. Additionally, every handle has a flag on it specifying if it is inheritable or not.

CoreFx has the bInheritHandles argument hard coded to true. While this feature can be useful, it can cause untold number of problems.

This of the situation where you have a file handle, and you've acquired that file handle with FileShare.None set. While the handle is opened, you start a child process. That child gets a copy of that handle, now they cannot use it because you've selected no sharing, but they have a copy of it. Since closing of file handles is a reference counting game, you're now unable to close the file. Sure, you can close your local handle, but you'll be unable to re-open it until the child process' handle is closed as well.

If the child spawns a child, the grand-child process will also keep a copy of the handle and could never get your file back.

This becomes worse in the case of redirected pipes. Imagine this setup:

  1. Thread A starts creating a new process with redirected output
  2. Thread B starts creating a new process designed as a long running service (think redis or something)
  3. Thread A creates the necessary handles to redirect output.
  4. Thread B calls Kernel32.dll!CreateProcess to create the process, the new process inherits the handles Thread A created for its child (but it doesn't know that and it cannot use them).
  5. Thread A calls Kernel32dll!CreateProcess.
  6. Thread A attempts to read standard output until it closes.
  7. Thread A child process exits, and closes the handles it was given BUT the pipes do not close because Thread B child process has a handle on them (and closing is a reference counting game).

Effectively, Thread A is deadlocked attempting to read from the stand output pipe of its child process (even though said process has exit already) until Thread B child process exits (and closes its handles).

This is a disaster. Handle inheritance should be take seriously, and it appears the CoreFx library isn't doing so. Ideally, the bInheritHandles parameter would be null unless STARTF_USESTDHANDLES is specified. When STARTF_USESTDHANDLES, then the advice of Raymon Chen should be followed: "The case of the redirected standard handles that won’t close even though the child process has exited".

There is an attempt to correct this bad behavior via a Monitor lock placed around the calls to CreateProcess calls. This is fine so long as all code in a process is running though CoreFx, but in any reasonable sized project that cannot be assumed.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions