Add Process.Start callback overload for advanced Interop

[NN---]

Background and motivation

There are plenty of ways to create a process where .NET does not support this or where there is already a process handle from native method.
Currently there is no possibility to create a .NET Process object from the SafeProcessHandle.

API Proposal

namespace System.Runtime.InteropServices;

[SupportedOSPlatform("windows")]
public ref struct WindowsProcessStartArguments
{
    public WindowsProcessStartArguments();

    public unsafe char* Arguments { get; }
    public unsafe char* EnvironmentVariables{ get; }

    public ProcessStartInfo ProcessStartInfo { get; }
    public nint StandardError { get; }
    public nint StandardInput { get; }
    public nint StandardOutput { get; }

    public static Process Start(ProcessStartInfo startInfo, Func<WindowsProcessStartArguments, SafeProcessHandle> callback);
}

[UnsupportedOSPlatform("windows")]
public ref struct UnixProcessStartArguments
{
    public UnixProcessStartArguments();

    public unsafe byte* ResolvedPath { get; }
    public unsafe byte** Arguments { get; }
    public unsafe byte** EnvironmentVariables { get; }

    public ProcessStartInfo ProcessStartInfo { get; }
    public nint StandardError { get; }
    public nint StandardInput { get; }
    public nint StandardOutput { get; }

    [UnsupportedOSPlatform("ios")]
    [UnsupportedOSPlatform("tvos")]
    [UnsupportedOSPlatform("maccatalyst")]
    public static Process Start(ProcessStartInfo startInfo, Func<UnixProcessStartArguments, int> callback);
}

API Usage

ProcessStartInfo startInfo = new("cmd")
{
    ArgumentList = { "/c", "echo hello && echo error 1>&2" },
    RedirectStandardOutput = true,
    RedirectStandardError = true
};

using Process process = WindowsProcessStartArguments.Start(startInfo, (WindowsProcessStartArguments args) =>
{
    Interop.Kernel32.STARTUPINFOEX startupInfoEx = default;
    Interop.Kernel32.PROCESS_INFORMATION processInfo = default;
    Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = default;

    startupInfoEx.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFOEX);
    startupInfoEx.StartupInfo.hStdInput = args.StandardInput;
    startupInfoEx.StartupInfo.hStdOutput = args.StandardOutput;
    startupInfoEx.StartupInfo.hStdError = args.StandardError;
    startupInfoEx.StartupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES;

    bool retVal = Interop.Kernel32.CreateProcess(
        null,
        args.Arguments,
        ref unused_SecAttrs,
        ref unused_SecAttrs,
        bInheritHandles: true,
        Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT,
        args.EnvironmentVariables,
        null,
        &startupInfoEx,
        &processInfo
    );

    if (!retVal)
    {
        throw new Win32Exception();
    }

    Interop.Kernel32.CloseHandle(processInfo.hThread);

    return new SafeProcessHandle(processInfo.hProcess, ownsHandle: true);
});

Alternative Designs

The alternative design included a ctor, but it was too fragile. Check #128491 for details

Risks

Can't see any as of now, as BCL takes care of removing all known risks before invoking the callback.

Working prototype: #128862

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

Issue Details

---

Background and motivation

There are plenty of ways to create a process where .NET does not support this or where there is already a process handle from native method.
Currently there is no possibility to create a .NET Process object from the SafeProcessHandle.

API Proposal

namespace System.Diagnostics;

public class Process
{
 public Process(SafeProcessHandle process)
}

API Usage

using SafeProcessHandle processHandle = Win32Api.CreateProcess(…);
Process process = new Process(processHandle);

Alternative Designs

No response

Risks

There should be no risk.
We already have a precedent with Socket constructor from SafeSockegHandle.

Author:	NN---
Assignees:	-
Labels:	api-suggestion, area-System.Diagnostics.Process
Milestone:	-

[KalleOlaviNiemitalo]

Would this be a Windows-only API? Not clear to me how you'd get a SafeProcessHandle from a native method on Linux or Mac.

[AraHaan]

I am sure there is a way yes since they too have process ID's.

[DaZombieKiller]

Would this be a Windows-only API? Not clear to me how you'd get a SafeProcessHandle from a native method on Linux or Mac.

On non-Windows, Process.SafeHandle will return a pseudo-SafeProcessHandle wrapping the process ID, but there's no way to get the wrapped process ID from the handle, making the pseudo-handle almost useless since you can't get back to the Process. I don't think you can get such an instance from a P/Invoke, though I've never tried.

[fbrosseau]

Would this be a Windows-only API? Not clear to me how you'd get a SafeProcessHandle from a native method on Linux or Mac.

Modern linux does have proper process file descriptors, but that feature may be too modern for dotnet support (I don't know?). They do solve many problems however - race conditions around PID recycling, etc.

[DaZombieKiller]

Thinking about this further, would it make more sense for this to be exposed as a static method like GetProcessById?

using Microsoft.Win32.SafeHandles;

namespace System.Diagnostics;

public partial class Process
{
    public static Process GetProcessByHandle(SafeProcessHandle handle);
    public static Process GetProcessByHandle(IntPtr handle); // potential extra API
}

Having a constructor would be consistent with Socket, but the static method is more consistent with the other APIs on Process. It'd also open up the opportunity for the Process objects to be cached, though I don't believe they are currently, even for GetProcessById.

[KalleOlaviNiemitalo]

For creating a Process from a native process handle (SafeProcessHandle or IntPtr) on Windows, I don't think caching would be a good idea. The handle might not have the same access rights as what the Process class specifies when it opens a process handle, and it might have access to a process that the current process is not normally authorised to open. If I created a Process object from such a handle, I would not want that to affect the behaviour of future Process.GetProcessById calls because such an effect would make my app more difficult to test. Also, I would not want Process.Dispose of a different process object to close my process handle prematurely.

Re implementing this on Linux, pidfd_open seems to be the function referenced in #71595 (comment). If I read this correctly, a pidfd does not carry any access rights (if you receive a pidfd, you cannot use that to kill a process that you wouldn't otherwise be authorised to kill).