diff --git a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs index 52dfab59a18f05..552e7367026faa 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -16,8 +16,6 @@ public SafeProcessHandle(System.IntPtr existingHandle, bool ownsHandle) : base ( public void Kill() { } public int ProcessId { get { throw null; } } protected override bool ReleaseHandle() { throw null; } - [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] - [System.Runtime.Versioning.SupportedOSPlatformAttribute("macos")] public void Resume() { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] @@ -356,8 +354,6 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< public System.Text.Encoding? StandardOutputEncoding { get { throw null; } set { } } public Microsoft.Win32.SafeHandles.SafeFileHandle? StandardOutputHandle { get { throw null; } set { } } public bool StartDetached { get { throw null; } set { } } - [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] - [System.Runtime.Versioning.SupportedOSPlatformAttribute("macos")] public bool StartSuspended { get { throw null; } set { } } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] public bool UseCredentialsForNetworkingOnly { get { throw null; } set { } } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index ad3ade2fa5b5b3..2f7fdacebcf0e3 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -210,20 +210,12 @@ internal static SafeProcessHandle Start(ProcessStartInfo startInfo, bool fallbac /// /// /// On Windows, this calls ResumeThread on the main thread of the process. - /// On macOS, this sends SIGCONT to the process. + /// On Unix, this sends SIGCONT to the process. /// /// The handle is invalid. - /// The current operating system is not supported. /// The OS call to resume the process failed. - [SupportedOSPlatform("windows")] - [SupportedOSPlatform("macos")] public void Resume() { - if (!OperatingSystem.IsWindows() && !OperatingSystem.IsMacOS()) - { - throw new PlatformNotSupportedException(); - } - Validate(); ResumeCore(); } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs index 96009d99e1054a..081e1bbe60a94a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs @@ -161,11 +161,13 @@ public string Arguments /// On macOS, the process is started with the POSIX_SPAWN_START_SUSPENDED flag. /// /// + /// On other Unix platforms, the child process raises SIGSTOP against itself before calling execve, + /// so the process has not yet transitioned to the target program image when it is suspended. + /// + /// /// This property cannot be used together with set to . /// /// - [SupportedOSPlatform("windows")] - [SupportedOSPlatform("macos")] public bool StartSuspended { get; set; } /// @@ -473,17 +475,10 @@ internal void ThrowIfInvalid(out bool anyRedirection, out SafeHandle[]? inherite throw new InvalidOperationException(SR.StartDetachedNotCompatible); } -#pragma warning disable CA1416 // StartSuspended getter works on all platforms; the attribute guards the actual effect - if (StartSuspended && !OperatingSystem.IsWindows() && !OperatingSystem.IsMacOS()) - { - throw new PlatformNotSupportedException(); - } - if (StartSuspended && UseShellExecute) { throw new InvalidOperationException(SR.StartSuspendedNotCompatible); } -#pragma warning restore CA1416 if (InheritedHandles is not null && (UseShellExecute || !string.IsNullOrEmpty(UserName))) { diff --git a/src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs b/src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs index d748ed1b272370..7d134824c76ac3 100644 --- a/src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs @@ -12,9 +12,21 @@ namespace System.Diagnostics.Tests { [ConditionalClass(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)] public class StartSuspendedTests : ProcessTestBase { + [ConditionalFact] + public void StartSuspended_ResumeImmediately_Succeeds() + { + using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + process.StartInfo.StartSuspended = true; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + processHandle.Resume(); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(WaitInMS)); + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + } + [ConditionalFact] public void StartSuspended_ResumeCompletes() { @@ -176,15 +188,4 @@ public async Task StartSuspended_WithPipeRedirection_Works() } } } - - public class StartSuspendedTests_NonWindowsNonMacOS : ProcessTestBase - { - [Fact] - [SkipOnPlatform(TestPlatforms.Windows | TestPlatforms.OSX | TestPlatforms.iOS | TestPlatforms.tvOS, "Resume is supported on Windows and macOS, and SafeProcessHandle.Open is not supported on iOS and tvOS")] - public void Resume_OnNonSupportedOS_ThrowsPlatformNotSupportedException() - { - using SafeProcessHandle handle = SafeProcessHandle.Open(Environment.ProcessId); - Assert.Throws(() => handle.Resume()); - } - } } diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index c5af90e397287e..6ec871286145ea 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -729,13 +729,6 @@ static int32_t ForkAndExecProcessInternal( #endif #if HAVE_FORK - if (startSuspended) - { - // POSIX_SPAWN_START_SUSPENDED is only available in the posix_spawn() path (macOS, !setCredentials). - // The fork() path does not support startSuspended. - errno = ENOTSUP; - return -1; - } bool success = true; int waitForChildToExecPipe[2] = {-1, -1}; pid_t processId = -1; @@ -811,11 +804,23 @@ static int32_t ForkAndExecProcessInternal( // on another thread while a vfork() child is still pending, bad things are possible; however we // do not do that. + // When startSuspended is true, we must use fork() instead of vfork() because we need to + // stop the child process before calling execve(), which is not safe with vfork() (the parent + // is blocked until the child calls execve or exits). + #if defined (__GLIBC__) - if ((processId = vfork()) == 0) // processId == 0 if this is child process + if (startSuspended) + { + processId = fork(); + } + else + { + processId = vfork(); + } + if (processId == 0) #else // musl libc has an undocumented failure mode around setuid(); we must exclude it. - if (setCredentials) + if (setCredentials || startSuspended) { processId = fork(); } @@ -933,6 +938,22 @@ static int32_t ForkAndExecProcessInternal( } #endif + // If startSuspended is requested, stop the child process before exec. + // The parent will send SIGCONT (via Resume()) when ready. + if (startSuspended) + { + // Close the write end of the exec-waiting pipe before stopping. + // This signals to the parent that the child has been set up successfully + // (no error written to the pipe), allowing the parent to return the child PID. + // When the child is later resumed with SIGCONT, it will proceed to execve. + close(waitForChildToExecPipe[WRITE_END_OF_PIPE]); + waitForChildToExecPipe[WRITE_END_OF_PIPE] = -1; + + // Send SIGSTOP to the current process. raise() sends the signal to the + // calling thread. This is safe because after fork the child is single-threaded. + raise(SIGSTOP); + } + // Finally, execute the new process. execve will not return if it's successful. execve(filename, argv, envp); ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); // execve failed @@ -974,6 +995,34 @@ done:; CloseIfOpen(waitForChildToExecPipe[READ_END_OF_PIPE]); } + // When startSuspended is requested, the child closes the exec-waiting pipe just before + // calling raise(SIGSTOP). Wait here until the child is truly in the stopped state so that + // the caller cannot send SIGCONT (via Resume()) before the child has delivered SIGSTOP. + // Without this synchronization, SIGCONT may arrive before SIGSTOP, causing the child to + // stop permanently on the subsequent SIGSTOP. + if (startSuspended && success) + { + int childStatus; + pid_t waitResult; + do + { + waitResult = waitpid(processId, &childStatus, WUNTRACED); + } + while (waitResult == -1 && errno == EINTR); + + if (waitResult == -1) + { + success = false; + priorErrno = errno; + } + else if (!WIFSTOPPED(childStatus)) + { + // Child exited (WIFEXITED) or was killed by a signal (WIFSIGNALED) before stopping. + success = false; + priorErrno = ECHILD; + } + } + // If we failed, give back error values in all out arguments. if (!success) {