Skip to content

Fix StartAndForget_StartsProcessAndReturnsValidPid by mapping ProcessStartInfo.Arguments for the string+args overload#127116

Merged
adamsitnik merged 7 commits intomainfrom
copilot/fix-flaky-test-starts-process
Apr 20, 2026
Merged

Fix StartAndForget_StartsProcessAndReturnsValidPid by mapping ProcessStartInfo.Arguments for the string+args overload#127116
adamsitnik merged 7 commits intomainfrom
copilot/fix-flaky-test-starts-process

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 19, 2026

StartAndForget_StartsProcessAndReturnsValidPid was failing intermittently for useProcessStartInfo: false because the test passed template.StartInfo.ArgumentList, while RemoteExecutor populates Arguments. This could launch dotnet with effectively empty/incorrect arguments and exit early.

  • What changed

    • Reverted the prior SIGKILL-based liveness workaround and restored the original HasExited assertion path and cleanup.
    • Updated the useProcessStartInfo: false branch to map template.StartInfo.Arguments into List<string> and pass that list to Process.StartAndForget(string, IList<string>?).
    • Added a local argument parsing helper in StartAndForget.cs (based on the referenced logic from PR Add new process management APIs to SafeProcessHandle #124375) to split the Arguments string into a list.
    • Inlined MapToArgumentList(template.StartInfo) directly in the StartAndForget call so mapping is only computed when useProcessStartInfo is false.
    • Added a clarifying comment above MapToArgumentList explaining why this helper is needed in this test (RemoteExecutor uses Arguments, while this overload requires a list).
  • Code shape

using Process template = CreateSleepProcess((int)TimeSpan.FromHours(1).TotalMilliseconds);
int pid = useProcessStartInfo
    ? Process.StartAndForget(template.StartInfo)
    : Process.StartAndForget(template.StartInfo.FileName, MapToArgumentList(template.StartInfo));

using Process launched = Process.GetProcessById(pid);
try
{
    Assert.False(launched.HasExited);
}
finally
{
    launched.Kill();
    launched.WaitForExit();
}
  • Validation
    • Ran targeted System.Diagnostics.Process tests for this area with no failures.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Copilot AI requested review from Copilot and removed request for Copilot April 19, 2026 11:32
Copilot AI requested review from Copilot and removed request for Copilot April 19, 2026 11:37
Copilot AI changed the title [WIP] Fix flaky test failure in StartAndForget_StartsProcessAndReturnsValidPid Stabilize StartAndForget_StartsProcessAndReturnsValidPid by replacing flaky HasExited check Apr 19, 2026
Copilot AI requested a review from adamsitnik April 19, 2026 11:38
Copy link
Copy Markdown
Member

@adamsitnik adamsitnik left a comment

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/System.Diagnostics.Process/tests/StartAndForget.cs Outdated
Comment thread src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs Outdated
Copilot AI requested review from Copilot and removed request for Copilot April 19, 2026 12:19
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 19, 2026

@copilot address my feedback

Addressed in b4ecac2. I applied the requested feedback to StartAndForget.cs and pushed the update.

Copilot AI requested a review from adamsitnik April 19, 2026 12:20
Comment thread src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs
Copy link
Copy Markdown
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

From the logs:

System.Diagnostics.Tests.StartAndForgetTests.StartAndForget_StartsProcessAndReturnsValidPid(useProcessStartInfo: False) [FAIL]
      Assert.True() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /_/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs(30,0): at System.Diagnostics.Tests.StartAndForgetTests.StartAndForget_StartsProcessAndReturnsValidPid(Boolean useProcessStartInfo)
           at System.Object.InvokeStub_StartAndForgetTests.StartAndForget_StartsProcessAndReturnsValidPid(Object, Span`1)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(95,0): at System.Reflection.MethodBaseInvoker.InvokeWithOneArg(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

It fails when useProcessStartInfo: False and I think I know why. RemoteExecutor is using dotnet to invoke the process using Arguments, not ArgumentsList. When we pass the ArgumentsList here:

: Process.StartAndForget(template.StartInfo.FileName, template.StartInfo.ArgumentList);

we pass an empty string, and invoke dotnet which prints sth like this:

PS C:\Users\adsitnik> dotnet

Usage: dotnet [path-to-application]
Usage: dotnet [commands]

path-to-application:
  The path to an application .dll file to execute.

commands:
  -h|--help                         Display help.
  --info                            Display .NET information.
  --list-runtimes [--arch <arch>]   Display the installed runtimes matching the host or specified architecture. Example architectures: arm64, x64, x86.
  --list-sdks [--arch <arch>]       Display the installed SDKs matching the host or specified architecture. Example architectures: arm64, x64, x86.

And exits. That is why the process is sometimes gone when we get to HasExited check.

@copilot please do the following:

  • revert all the changes you have done here in this PR so far
  • take this logic from #124375 and simply map template.StartInfo to List<string> and pass them to StartAndForget

@adamsitnik adamsitnik marked this pull request as ready for review April 19, 2026 20:55
Copilot AI review requested due to automatic review settings April 19, 2026 20:55
Copy link
Copy Markdown
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes intermittent failures in StartAndForget_StartsProcessAndReturnsValidPid by ensuring the string + args overload receives arguments derived from ProcessStartInfo.Arguments (which RemoteExecutor populates), rather than relying on ArgumentList (which may be empty).

Changes:

  • Maps ProcessStartInfo.Arguments into a List<string> for the Process.StartAndForget(string, IList<string>?) path.
  • Adds a local helper to split the Arguments string into an argument list for the test.

Comment thread src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs
Comment thread src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs Outdated
Comment thread src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs
auto-merge was automatically disabled April 20, 2026 16:14

Head branch was pushed to by a user without write access

Copilot AI requested a review from jkotas April 20, 2026 16:15
Copilot AI requested review from Copilot and removed request for Copilot April 20, 2026 16:54
@adamsitnik adamsitnik enabled auto-merge (squash) April 20, 2026 18:56
@adamsitnik adamsitnik merged commit a8579d5 into main Apr 20, 2026
93 of 97 checks passed
@adamsitnik adamsitnik deleted the copilot/fix-flaky-test-starts-process branch April 20, 2026 20:26
@adamsitnik adamsitnik added this to the 11.0.0 milestone Apr 20, 2026
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.

Flaky test failure in at StartAndForget_StartsProcessAndReturnsValidPid

5 participants