Skip to content

Add macOS support for StartSuspended via POSIX_SPAWN_START_SUSPENDED#129570

Open
Copilot wants to merge 8 commits into
mainfrom
copilot/start-suspended-for-macos
Open

Add macOS support for StartSuspended via POSIX_SPAWN_START_SUSPENDED#129570
Copilot wants to merge 8 commits into
mainfrom
copilot/start-suspended-for-macos

Conversation

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Extends ProcessStartInfo.StartSuspended and SafeProcessHandle.Resume() to macOS using POSIX_SPAWN_START_SUSPENDED. Approved API: #94127 (comment)

Changes

  • API surface: Added [SupportedOSPlatform("macos")] to StartSuspended and Resume() in ref and src
  • Managed → native interop: Interop.Sys.ForkAndExecProcess managed wrapper now accepts ProcessStartInfo directly (instead of individual bool parameters) and extracts StartDetached, KillOnParentExit, and StartSuspended internally
  • Native (pal_process.c): Set POSIX_SPAWN_START_SUSPENDED flag in the macOS posix_spawn path when startSuspended is true; returns ENOTSUP in the fork() path when startSuspended is true to prevent silent behavior differences when credentials force the fork path on macOS
  • Native (pal_process_wasi.c): Updated WASI stub signature to include startSuspended parameter
  • Resume on Unix: ResumeCore calls SignalCore(PosixSignal.SIGCONT); shared Resume() throws PlatformNotSupportedException on unsupported OS; Resume() no longer prohibits multiple calls — OS errors are surfaced directly as Win32Exception
  • Validation: ProcessStartInfo.Validate() throws PlatformNotSupportedException when StartSuspended=true on non-Windows/non-macOS platforms; throws InvalidOperationException for UseShellExecute incompatibility on supported platforms
  • XML docs: Updated Resume() docs to remove the "can only be called once" restriction and document per-platform behavior
  • Tests: Extended StartSuspendedTests from TestPlatforms.Windows to TestPlatforms.Windows | TestPlatforms.OSX; Resume_OnNonSuspendedProcess test (Windows-only) now expects Win32Exception from the OS instead of InvalidOperationException; non-supported OS test verifies PlatformNotSupportedException

Usage

Process process = new()
{
    StartInfo =
    {
        FileName = "/usr/bin/some-app",
        StartSuspended = true, // now works on macOS
    }
};

using SafeProcessHandle handle = SafeProcessHandle.Start(process.StartInfo);
// process is suspended, configure it as needed...
handle.Resume(); // sends SIGCONT on macOS
ProcessExitStatus status = handle.WaitForExit();

Copilot AI and others added 2 commits June 18, 2026 10:21
…and Resume via SIGCONT

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI self-assigned this Jun 18, 2026
Copilot AI review requested due to automatic review settings June 18, 2026 10:44
Copilot AI removed the request for review from Copilot June 18, 2026 10:44
Copilot AI requested a review from adamsitnik June 18, 2026 10:45

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Comment thread src/native/libs/System.Native/pal_process.h
Comment thread src/native/libs/System.Native/pal_process.c Outdated
Comment thread src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs Outdated
Comment thread src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs Outdated
Comment thread src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs Outdated
Comment thread src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs Outdated
…plify ResumeCore, update WASI stub, fix tests

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 18, 2026 11:31

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please address my feedback

All feedback addressed in commit 3e2d0d6:

  • Passed ProcessStartInfo to Interop.Sys.ForkAndExecProcess instead of individual parameters
  • Simplified ResumeCore to just SignalCore(PosixSignal.SIGCONT), removed _startedSuspended field
  • Added platform check in shared Resume() to throw PlatformNotSupportedException on unsupported OS
  • Updated pal_process_wasi.c stub with startSuspended parameter
  • Fixed comment in fork() path (removed Catalyst mention)
  • Removed double-resume test, made Resume_OnNonSuspendedProcess Windows-specific, fixed non-supported OS test to expect PlatformNotSupportedException

All 635 tests pass with 0 failures.

Copilot AI requested a review from adamsitnik June 18, 2026 11:33

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

Comment thread src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs Outdated
…idate() before ResumeCore

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 18, 2026 11:58

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address my feedback

Both changes applied in the latest commit: reformatted the ForkAndExecProcess call to the preferred style and added Validate() before ResumeCore().

Copilot AI requested a review from adamsitnik June 18, 2026 11:58

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback and run the tests locally before pushing the changes

Comment thread src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs Outdated
@adamsitnik

Copy link
Copy Markdown
Member

/azp list

@azure-pipelines

Copy link
Copy Markdown
CI/CD Pipelines for this repository:

@adamsitnik

Copy link
Copy Markdown
Member

/azp run runtime-maccatalyst

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamsitnik adamsitnik requested review from sebastienros and wfurt June 18, 2026 18:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/libraries/System.Diagnostics.Process/tests/StartSuspendedTests.cs:66

  • The test suite no longer validates Resume()’s documented error behavior on macOS: the “Resume called twice” test was removed entirely, and Resume_OnNonSuspendedProcess_ThrowsInvalidOperationException is now Windows-only. Given the public XML docs for SafeProcessHandle.Resume() say it throws when the process wasn’t started suspended or has already been resumed, we should keep coverage for these cases on macOS once the implementation enforces the same contract there.
        [ConditionalFact]
        [PlatformSpecific(TestPlatforms.Windows)]
        public void Resume_OnNonSuspendedProcess_ThrowsInvalidOperationException()
        {
            Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode);

            using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo);

            Assert.Throws<InvalidOperationException>(() => processHandle.Resume());

            processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(WaitInMS));
        }

Comment on lines 476 to 479
if ((OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) && StartSuspended && UseShellExecute)
{
throw new InvalidOperationException(SR.StartSuspendedNotCompatible);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply this suggestion

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in the latest commit. ProcessStartInfo.Validate() now throws PlatformNotSupportedException when StartSuspended=true on non-Windows/non-macOS platforms, before checking the UseShellExecute incompatibility.

Comment thread src/native/libs/System.Native/pal_process.c
Comment thread src/native/libs/System.Native/pal_process_wasi.c

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Comment on lines 476 to 479
if ((OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) && StartSuspended && UseShellExecute)
{
throw new InvalidOperationException(SR.StartSuspendedNotCompatible);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply this suggestion

Comment thread src/native/libs/System.Native/pal_process.c
…nded on unsupported OS, update Resume() docs and Windows impl, return error in fork() path

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please address my feedback

All three items addressed in the latest commit:

  1. ProcessStartInfo.cs: Throws PlatformNotSupportedException when StartSuspended=true on non-Windows/non-macOS platforms
  2. SafeProcessHandle.cs XML docs: Updated to remove "can only be called once" restriction; Resume() now documents that OS errors are surfaced via Win32Exception
  3. pal_process.c fork() path: Returns ENOTSUP when startSuspended=true to prevent silent behavior differences when credentials force the fork() path on macOS

Copilot AI requested a review from adamsitnik June 22, 2026 14:44

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my feedback

…ndle after resume

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 22, 2026 15:41

Copilot AI commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address my feedback

Addressed in the latest commit — ResumeCore keeps the _mainThreadHandle null check (throws InvalidOperationException) but no longer disposes the handle after first resume.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API Proposal]: ProcessStartInfo.CreateSuspended, Process.Resume

3 participants