-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Add Process constructor accepting SafeProcessHandle and optional stream handles #128491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
3a6fc11
157b2ee
a53a1fc
a6488fc
a4e5588
b7cc712
41ae4c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,9 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections.Generic; | ||
| using System.ComponentModel; | ||
| using System.IO; | ||
| using System.IO.Pipes; | ||
| using System.Threading.Tasks; | ||
| using System.Runtime.InteropServices; | ||
| using Microsoft.DotNet.RemoteExecutor; | ||
| using Microsoft.DotNet.XUnitExtensions; | ||
| using Microsoft.Win32.SafeHandles; | ||
| using Xunit; | ||
|
|
||
|
|
@@ -27,7 +23,7 @@ public void ProcessStartedWithInvalidHandles_ConsoleReportsInvalidHandles() | |
| return RemoteExecutor.SuccessExitCode; | ||
| }); | ||
|
|
||
| Assert.Equal(RemoteExecutor.SuccessExitCode, RunWithInvalidHandles(process.StartInfo)); | ||
| Assert.Equal(RemoteExecutor.SuccessExitCode, RunWithHandles(process.StartInfo, (nint)(-1), (nint)(-1), (nint)(-1))); | ||
|
adamsitnik marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] | ||
|
|
@@ -64,7 +60,7 @@ public void ProcessStartedWithInvalidHandles_CanStartChildProcessWithDerivedInva | |
| } | ||
| }, restrictHandles.ToString(), killOnParentExit.ToString()); | ||
|
|
||
| Assert.Equal(RemoteExecutor.SuccessExitCode, RunWithInvalidHandles(process.StartInfo)); | ||
| Assert.Equal(RemoteExecutor.SuccessExitCode, RunWithHandles(process.StartInfo, (nint)(-1), (nint)(-1), (nint)(-1))); | ||
| } | ||
|
|
||
| [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] | ||
|
|
@@ -102,13 +98,110 @@ public void ProcessStartedWithInvalidHandles_CanRedirectOutput(bool restrictHand | |
| } | ||
| }, restrictHandles.ToString()); | ||
|
|
||
| Assert.Equal(RemoteExecutor.SuccessExitCode, RunWithInvalidHandles(process.StartInfo)); | ||
| Assert.Equal(RemoteExecutor.SuccessExitCode, RunWithHandles(process.StartInfo, (nint)(-1), (nint)(-1), (nint)(-1))); | ||
| } | ||
|
|
||
| private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) | ||
| [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] | ||
| public unsafe void ProcessStartedWithAnonymousPipeHandles_CanCaptureOutput() | ||
| { | ||
| const nint INVALID_HANDLE_VALUE = -1; | ||
| SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle outputReadHandle, out SafeFileHandle outputWriteHandle); | ||
| SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle errorReadHandle, out SafeFileHandle errorWriteHandle); | ||
|
adamsitnik marked this conversation as resolved.
Outdated
|
||
|
|
||
| using (outputReadHandle) | ||
| using (outputWriteHandle) | ||
| using (errorReadHandle) | ||
| using (errorWriteHandle) | ||
| { | ||
| // Enable inheritance on the write ends so the child process can use them. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is robust only if you control all other process starts that may be done by the current process. If I am reading this correctly, this will create inheritable handles in the main test process without synchronizing with other process starts and so the handles may get accidently inherited by other processes. Is this test going to be flaky because of that? Also, do the docs need to mention all gotchas about inherited handles and how the user code is expected to deal with them? I am not sure whether this API is the best way to address the end-to-end scenario once you take all the gotchas into account. |
||
| if (!Interop.Kernel32.SetHandleInformation( | ||
| outputWriteHandle, | ||
| Interop.Kernel32.HandleFlags.HANDLE_FLAG_INHERIT, | ||
| Interop.Kernel32.HandleFlags.HANDLE_FLAG_INHERIT)) | ||
| { | ||
| throw new Win32Exception(Marshal.GetLastWin32Error()); | ||
| } | ||
|
|
||
| if (!Interop.Kernel32.SetHandleInformation( | ||
| errorWriteHandle, | ||
| Interop.Kernel32.HandleFlags.HANDLE_FLAG_INHERIT, | ||
| Interop.Kernel32.HandleFlags.HANDLE_FLAG_INHERIT)) | ||
| { | ||
| throw new Win32Exception(Marshal.GetLastWin32Error()); | ||
| } | ||
|
|
||
| using Process remoteProcess = CreateProcess(() => | ||
| { | ||
| Console.Write("stdout_hello"); | ||
| Console.Error.Write("stderr_hello"); | ||
|
|
||
| return RemoteExecutor.SuccessExitCode; | ||
| }); | ||
|
|
||
| ProcessStartInfo startInfo = remoteProcess.StartInfo; | ||
| string arguments = $"\"{startInfo.FileName}\" {startInfo.Arguments}\0"; | ||
|
|
||
| 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 = (nint)(-1); | ||
| startupInfoEx.StartupInfo.hStdOutput = outputWriteHandle.DangerousGetHandle(); | ||
| startupInfoEx.StartupInfo.hStdError = errorWriteHandle.DangerousGetHandle(); | ||
| startupInfoEx.StartupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; | ||
|
|
||
| fixed (char* commandLinePtr = arguments) | ||
| { | ||
| bool retVal = Interop.Kernel32.CreateProcess( | ||
| null, | ||
| commandLinePtr, | ||
| ref unused_SecAttrs, | ||
| ref unused_SecAttrs, | ||
| bInheritHandles: true, | ||
| Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT, | ||
| null, | ||
| null, | ||
| &startupInfoEx, | ||
| &processInfo | ||
| ); | ||
|
|
||
| if (!retVal) | ||
| { | ||
| throw new Win32Exception(); | ||
| } | ||
| } | ||
|
|
||
| try | ||
| { | ||
| SafeProcessHandle safeProcessHandle = new(processInfo.hProcess, ownsHandle: true); | ||
|
adamsitnik marked this conversation as resolved.
Outdated
|
||
|
|
||
| using Process process = new( | ||
| safeProcessHandle, | ||
| standardOutput: outputReadHandle, | ||
| standardError: errorReadHandle); | ||
|
|
||
| // Close the write ends so reads don't block once the child exits. | ||
| outputWriteHandle.Close(); | ||
| errorWriteHandle.Close(); | ||
|
|
||
| string stdout = process.StandardOutput.ReadToEnd(); | ||
| string stderr = process.StandardError.ReadToEnd(); | ||
|
adamsitnik marked this conversation as resolved.
Outdated
|
||
|
|
||
| process.WaitForExit(WaitInMS); | ||
|
|
||
| Assert.Equal("stdout_hello", stdout); | ||
| Assert.Equal("stderr_hello", stderr); | ||
| Assert.Equal(RemoteExecutor.SuccessExitCode, process.ExitCode); | ||
| } | ||
| finally | ||
| { | ||
| Interop.Kernel32.CloseHandle(processInfo.hThread); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private unsafe int RunWithHandles(ProcessStartInfo startInfo, nint hStdInput, nint hStdOutput, nint hStdError, bool inheritHandles = false) | ||
| { | ||
| // RemoteExector has provided us with the right path and arguments, | ||
| // we just need to add the terminating null character. | ||
| string arguments = $"\"{startInfo.FileName}\" {startInfo.Arguments}\0"; | ||
|
|
@@ -118,9 +211,9 @@ private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) | |
| Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = default; | ||
|
|
||
| startupInfoEx.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFOEX); | ||
| startupInfoEx.StartupInfo.hStdInput = INVALID_HANDLE_VALUE; | ||
| startupInfoEx.StartupInfo.hStdOutput = INVALID_HANDLE_VALUE; | ||
| startupInfoEx.StartupInfo.hStdError = INVALID_HANDLE_VALUE; | ||
| startupInfoEx.StartupInfo.hStdInput = hStdInput; | ||
| startupInfoEx.StartupInfo.hStdOutput = hStdOutput; | ||
| startupInfoEx.StartupInfo.hStdError = hStdError; | ||
|
|
||
| // If STARTF_USESTDHANDLES is not set, the new process will inherit the standard handles. | ||
| startupInfoEx.StartupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; | ||
|
|
@@ -133,7 +226,7 @@ private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) | |
| commandLinePtr, | ||
| ref unused_SecAttrs, | ||
| ref unused_SecAttrs, | ||
| bInheritHandles: false, | ||
| bInheritHandles: inheritHandles, | ||
| Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT, | ||
| null, | ||
| null, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.