Make Process.Start have a option to change handle inheritance

[pdelvo]

Currently if you call Process.Start internally CreateProcess is called with bInheritHandles = true (hard coded). It would be great to make it possible to change this behavior, e.g. by adding a Property to ProcessStartInfo.

Currently there is no way I know of to change this other then reimplementing System.Diagnostics.Process.

Example

If you run this application twice without exiting the first notepad instance the second instance will not be able to open the tcp port, because notepad is still running. This can be a problem for server applications that are starting child processes themself and crash, or are killed by the user before the socket can be closed.

using System.Diagnostics;
using System.Net;
using System.Net.Sockets;

class Program
{
    static void Main()
    {
        TcpListener listener = new TcpListener(IPAddress.Any, 4567);
        listener.Start();

        Process.Start(new ProcessStartInfo("notepad.exe") { UseShellExecute = false });
        //Simulate application crash without freeing resources
    }
}

Design proposal

The easiest way to make this possible is to add a new Property to ProcessStartInfo and use this in the Call to CreateProcess

namespace System.Diagnostics
{
    public partial class ProcessStartInfo
    {
        public bool InheritHandles
        {
            get;  // defaults to true
            [MinimumOSPlatform("windows7.0")]
            set;
        }
    }
}

Questions

• Is there a very important reason why this was hardcoded like this in the first place?

[krwq]

I agree we should add this API. IMO we should also make TcpListener have option to change HandleInheritability and change the default behavior to not inherit handles if possible (at least when targeting newer version of the framework).

In most cases we should not change InheritHandles on Process and should rather do it whenever we create handles but we should still have an option to disable this.

[pdelvo]

I think adding options to TcpListener/TcpClient/Socket would be a good idea. Are there other places where handle inheritance can be a problem? I was not able to create a problem like the one mentioned above by exclusivly opening a file

[terrajobst]

This seems quite reasonable. As @krwq we should probably do a pass and look into similar classes that perform handle inheritability. This helps in understanding what API we could use.

[krwq]

@pdelvo, We were able to repro this with FileStream although that one creates non-inheritable handles by default so had to override that. I believe that every library should have non-inheritable handles by default but that might be hard to change at this point because of compat reasons

[ellismg]

One open question I have is what this would mean when we try to support this feature cross platform. What would it mean to say that handles are not inherited in Unix? Does "handles" here mean open file descriptors? If they are not to be inherited, does the framework have to find and close them in the child process? How does it interact with file descriptors marked FD_CLOEXEC?

[stephentoub]

I expect there are going to be a fair number of features in System.Diagnostics.Process that result in PlatformNotSupportedException; this might be one of them, if e.g. explicitly closing all fds above 2 doesn't work out or is prohibitive for some reason.

[terrajobst]

We've reviewed this proposal and don't believe it's ready yet. Please take a look at the notes.

[danmoseley]

Fixed link: https://github.com/dotnet/apireviews/tree/master/2015/01-14-misc#306-make-processstart-have-a-option-to-change-handle-inheritance

[Priya91]

@pdelvo @whoisj Would one of you be interested in working on the api proposal for this, responding to the last api review in @danmosemsft's link..

[whoisj]

@Priya91 yeah I could do something, but likely not until after VS 2017 ships (it is an all consuming effort). If @pdelvo wants to start on something, I'd be happy to collaborate as well.

What are the requirements for CoreFx API changes? Does the API need x-plat support, can there be Windows specific elements? Etc.

As a side note, I'd prefer to see a method added to System.Diagnostics.ProcessStartInfo like public void AddInhertiableHandles(IEnumerable<SafeHandle> handles), public void AddInhertiableHandles(IEnumerable<IntPtr> handles) than a misleading property like public bool InheritHandles { get; set; } because any time there's a redirection of pipes, inheritance needs to be enabled but consumers of the API do not likely mean to inherit every handle the parent process has.

[Priya91]

What are the requirements for CoreFx API changes? Does the API need x-plat support

We have the api addition process documented here. It also elaborates on the design principles. Yeah we do need x-plat support, as .NET Core supports Unix platforms as well.

can there be Windows specific elements? Etc.

Are you asking in terms of exposing an API only on Windows? We can't do that, although we have some windows specific APIs in .NET Core and throw PNSE on other platforms.

For now the focus should be more on the API design as you suggested in using a method over property etc, which will take shape once we understand the scenarios and requirements on all platforms.

[whoisj]

@Priya91 👍 and thanks.