-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Add Process.Start callback overload with ref struct ProcessStartArguments #128862
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 11 commits
f1c70ae
59234fd
3163021
ba27166
9c12e8e
8b593b3
d01ae8e
b29a721
03cda31
090d716
cf52098
2245788
d46d4bc
3079bdd
26a3800
d30a6f6
39ef8d1
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 |
|---|---|---|
|
|
@@ -215,6 +215,11 @@ public void Refresh() { } | |
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] | ||
| [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] // this needs to come after the ios attribute due to limitations in the platform analyzer | ||
| public static System.Diagnostics.Process? Start(System.Diagnostics.ProcessStartInfo startInfo) { throw null; } | ||
| [System.CLSCompliantAttribute(false)] | ||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] | ||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] | ||
| [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] | ||
| public static System.Diagnostics.Process Start(System.Diagnostics.ProcessStartInfo startInfo, System.Func<System.Diagnostics.ProcessStartArguments, Microsoft.Win32.SafeHandles.SafeProcessHandle> callback) { throw null; } | ||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] | ||
|
adamsitnik marked this conversation as resolved.
|
||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] | ||
| [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] // this needs to come after the ios attribute due to limitations in the platform analyzer | ||
|
|
@@ -287,6 +292,19 @@ public readonly partial struct ProcessOutputLine | |
| public bool StandardError { get { throw null; } } | ||
| public override string ToString() { throw null; } | ||
| } | ||
| public sealed partial class ProcessStartArguments | ||
|
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. Something to consider: Should this be
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. Sounds very reasonable. But this would force us to introduce a dedicated
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. Yes - if we wanted to pass the structs as
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. @copilot please give this idea a try (make it
Contributor
Author
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. Done in |
||
| { | ||
| public ProcessStartArguments() { } | ||
|
adamsitnik marked this conversation as resolved.
|
||
| [System.CLSCompliantAttribute(false)] | ||
| public unsafe void* Arguments { get { throw null; } set { } } | ||
| [System.CLSCompliantAttribute(false)] | ||
| public unsafe void* EnvironmentVariables { get { throw null; } set { } } | ||
| public string? FileName { get { throw null; } set { } } | ||
| public System.Diagnostics.ProcessStartInfo ProcessStartInfo { get { throw null; } set { } } | ||
| public System.IntPtr StandardError { get { throw null; } set { } } | ||
| public System.IntPtr StandardInput { get { throw null; } set { } } | ||
| public System.IntPtr StandardOutput { get { throw null; } set { } } | ||
| } | ||
| public enum ProcessPriorityClass | ||
| { | ||
| Normal = 32, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -218,6 +218,118 @@ internal static SafeProcessHandle StartCore(ProcessStartInfo startInfo, SafeFile | |
| out waitStateHolder); | ||
| } | ||
|
|
||
| internal static unsafe SafeProcessHandle StartWithCallback(ProcessStartInfo startInfo, SafeFileHandle? stdinFd, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, | ||
| Func<ProcessStartArguments, SafeProcessHandle> callback, out ProcessWaitState.Holder? waitStateHolder) | ||
| { | ||
| waitStateHolder = null; | ||
| ProcessUtils.EnsureInitialized(); | ||
|
|
||
| string? resolvedFileName = ProcessUtils.ResolvePath(startInfo.FileName); | ||
| if (string.IsNullOrEmpty(resolvedFileName)) | ||
| { | ||
| Interop.ErrorInfo error = Interop.Error.ENOENT.Info(); | ||
| throw ProcessUtils.CreateExceptionForErrorStartingProcess(error.GetErrorMessage(), error.RawErrno, startInfo.FileName, startInfo.WorkingDirectory); | ||
| } | ||
|
|
||
|
adamsitnik marked this conversation as resolved.
|
||
| string[] argv = ProcessUtils.ParseArgv(startInfo); | ||
| bool configuredTerminal = false, usesTerminal = UsesTerminal(stdinFd, stdoutHandle, stderrHandle); | ||
| byte** argvPtr = null, envpPtr = null; | ||
| bool stdinRefAdded = false, stdoutRefAdded = false, stderrRefAdded = false; | ||
|
|
||
| try | ||
| { | ||
| Interop.Sys.AllocArgvArray(argv, ref argvPtr); | ||
| Interop.Sys.AllocEnvpArray(startInfo.Environment, ref envpPtr); | ||
|
|
||
| int stdinRawFd = -1, stdoutRawFd = -1, stderrRawFd = -1; | ||
|
|
||
| if (stdinFd is not null) | ||
| { | ||
| stdinFd.DangerousAddRef(ref stdinRefAdded); | ||
| stdinRawFd = stdinFd.DangerousGetHandle().ToInt32(); | ||
|
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. These |
||
| } | ||
|
|
||
| if (stdoutHandle is not null) | ||
| { | ||
| stdoutHandle.DangerousAddRef(ref stdoutRefAdded); | ||
| stdoutRawFd = stdoutHandle.DangerousGetHandle().ToInt32(); | ||
| } | ||
|
|
||
| if (stderrHandle is not null) | ||
| { | ||
| stderrHandle.DangerousAddRef(ref stderrRefAdded); | ||
| stderrRawFd = stderrHandle.DangerousGetHandle().ToInt32(); | ||
| } | ||
|
|
||
| ProcessStartArguments args = new() | ||
| { | ||
| FileName = resolvedFileName, | ||
| Arguments = argvPtr, | ||
| EnvironmentVariables = envpPtr, | ||
| StandardInput = stdinRawFd, | ||
| StandardOutput = stdoutRawFd, | ||
| StandardError = stderrRawFd, | ||
| ProcessStartInfo = startInfo, | ||
| }; | ||
|
|
||
| // Lock to avoid races with OnSigChild | ||
| // By using a ReaderWriterLock we allow multiple processes to start concurrently. | ||
| ProcessUtils.s_processStartLock.EnterReadLock(); | ||
|
|
||
| try | ||
| { | ||
| if (usesTerminal) | ||
| { | ||
| ProcessUtils.ConfigureTerminalForChildProcesses(1); | ||
| configuredTerminal = true; | ||
| } | ||
|
|
||
| SafeProcessHandle processHandle = callback(args); | ||
| if (processHandle is null || processHandle.IsInvalid) | ||
| { | ||
| throw new ArgumentException(SR.Argument_InvalidHandle, nameof(callback)); | ||
| } | ||
|
|
||
| waitStateHolder = new ProcessWaitState.Holder(processHandle.ProcessId, isNewChild: true, usesTerminal); | ||
| return processHandle; | ||
|
adamsitnik marked this conversation as resolved.
Outdated
|
||
| } | ||
| finally | ||
| { | ||
| ProcessUtils.s_processStartLock.ExitReadLock(); | ||
| } | ||
|
adamsitnik marked this conversation as resolved.
Outdated
|
||
| } | ||
| catch | ||
| { | ||
| if (configuredTerminal) | ||
| { | ||
| // We failed to launch a child that could use the terminal. | ||
| ProcessUtils.s_processStartLock.EnterWriteLock(); | ||
| ProcessUtils.ConfigureTerminalForChildProcesses(-1); | ||
| ProcessUtils.s_processStartLock.ExitWriteLock(); | ||
| } | ||
|
|
||
| throw; | ||
| } | ||
| finally | ||
| { | ||
| if (stdinRefAdded) | ||
| { | ||
| stdinFd!.DangerousRelease(); | ||
| } | ||
| if (stdoutRefAdded) | ||
| { | ||
| stdoutHandle!.DangerousRelease(); | ||
| } | ||
| if (stderrRefAdded) | ||
| { | ||
| stderrHandle!.DangerousRelease(); | ||
| } | ||
|
|
||
| NativeMemory.Free(envpPtr); | ||
| NativeMemory.Free(argvPtr); | ||
| } | ||
| } | ||
|
|
||
| private static SafeProcessHandle StartWithShellExecute(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, | ||
| SafeFileHandle? stderrHandle, out ProcessWaitState.Holder? waitStateHolder) | ||
| { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.