From 200ea87f7d1a41d3dfe5479a48a24e1a8a882739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Fri, 17 Apr 2026 18:18:48 +0200 Subject: [PATCH 1/5] Rework SignalHandler_CanDisposeInHandler to utilize RemoteInvokeHandle analytics on Dispose --- .../tests/ProcessTests.cs | 60 ++++++++++++++----- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 61dad866b71209..4f4c67b6882404 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -10,6 +10,7 @@ using System.Linq; using System.Net; using System.Reflection; +using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; using System.Security; using System.Text; @@ -194,14 +195,23 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) const string PosixSignalHandlerDisposedMessage = "PosixSignalRegistration disposed."; const int UnterminatedExitCode = -1; - var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false }; + // Process operations timeout cascading: + // WaitInMS * 1: Remote process wait for second signal after unregistering exercised signal handler + // WaitInMS * 2: RemoteExecutor Timeout, collects data and makes dump if not exited gracefully before + + var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false, TimeOut = WaitInMS * 2 }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; if (OperatingSystem.IsWindows()) { remoteInvokeOptions.StartInfo.CreateNewProcessGroup = true; + remoteInvokeOptions.CheckExitCode = true; + remoteInvokeOptions.ExpectedExitCode = unchecked((int)0xC000013A); // STATUS_CONTROL_C_EXIT } - using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + bool remoteHandleDisposed = false; + ExceptionDispatchInfo testException = null; + ExceptionDispatchInfo disposeException = null; + RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( (signalStr) => { PosixSignal expectedSignal = Enum.Parse(signalStr); @@ -238,7 +248,6 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) arg: $"{signal}", remoteInvokeOptions); - try { AssertRemoteProcessStandardOutputLine(remoteHandle, PosixSignalRegistrationCreatedMessage, WaitInMS); @@ -260,25 +269,46 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) SendSignal(signal, remoteHandle.Process); } - Assert.True(remoteHandle.Process.WaitForExit(WaitInMS)); - Assert.True(remoteHandle.Process.StandardOutput.EndOfStream); - - if (OperatingSystem.IsWindows()) + // For Windows, we prefer more feature rich termination and status checking in remoteHandle.Dispose() + // However on other platforms, exit code is platform dependent, so we check manually. + if (!OperatingSystem.IsWindows()) { - Assert.Equal(unchecked((int)0xC000013A), remoteHandle.Process.ExitCode); // STATUS_CONTROL_C_EXIT - } - else - { - // Signal numbers are platform dependent, so we can't check exact exit code + Assert.True(remoteHandle.Process.WaitForExit(WaitInMS)); + Assert.NotEqual(0, remoteHandle.Process.ExitCode); Assert.NotEqual(UnterminatedExitCode, remoteHandle.Process.ExitCode); + Assert.NotEqual(RemoteExecutor.SuccessExitCode, remoteHandle.Process.ExitCode); } + + remoteHandleDisposed = true; + remoteHandle.Dispose(); + } + catch (Exception ex) + { + testException = ExceptionDispatchInfo.Capture(ex); } finally { - // If sending the signal fails or process did not exit on its own, we want to kill the process ASAP - // to prevent RemoteExecutor's timeout from hiding it. - remoteHandle.Process.Kill(); + if (!remoteHandleDisposed) + { + try + { + remoteHandle.Dispose(); + } + catch (Exception ex) + { + disposeException = ExceptionDispatchInfo.Capture(ex); + } + } + } + // test exception is more important, exception at dispose could be consequence of it + if (testException is not null) + { + testException.Throw(); + } + if (disposeException is not null) + { + disposeException.Throw(); } } From 3e8894a053e38f8e205a385c02e83805c3b42b5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Mon, 20 Apr 2026 12:35:05 +0200 Subject: [PATCH 2/5] Variable rename --- .../System.Diagnostics.Process/tests/ProcessTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 4f4c67b6882404..1f7f638dfe2cb2 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -208,7 +208,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) remoteInvokeOptions.ExpectedExitCode = unchecked((int)0xC000013A); // STATUS_CONTROL_C_EXIT } - bool remoteHandleDisposed = false; + bool remoteHandleDisposeAttemped = false; ExceptionDispatchInfo testException = null; ExceptionDispatchInfo disposeException = null; RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( @@ -280,7 +280,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) Assert.NotEqual(RemoteExecutor.SuccessExitCode, remoteHandle.Process.ExitCode); } - remoteHandleDisposed = true; + remoteHandleDisposeAttemped = true; remoteHandle.Dispose(); } catch (Exception ex) @@ -289,7 +289,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) } finally { - if (!remoteHandleDisposed) + if (!remoteHandleDisposeAttemped) { try { From 99732a96b123263982d99e2f0f9d7b8a2f1b17a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Mon, 20 Apr 2026 16:02:02 +0200 Subject: [PATCH 3/5] Fix variable name typo --- .../System.Diagnostics.Process/tests/ProcessTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 1f7f638dfe2cb2..fc13f08c03cadc 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -208,7 +208,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) remoteInvokeOptions.ExpectedExitCode = unchecked((int)0xC000013A); // STATUS_CONTROL_C_EXIT } - bool remoteHandleDisposeAttemped = false; + bool remoteHandleDisposeAttempted = false; ExceptionDispatchInfo testException = null; ExceptionDispatchInfo disposeException = null; RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( @@ -280,7 +280,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) Assert.NotEqual(RemoteExecutor.SuccessExitCode, remoteHandle.Process.ExitCode); } - remoteHandleDisposeAttemped = true; + remoteHandleDisposeAttempted = true; remoteHandle.Dispose(); } catch (Exception ex) @@ -289,7 +289,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) } finally { - if (!remoteHandleDisposeAttemped) + if (!remoteHandleDisposeAttempted) { try { From 379a2a9b7de686dabce57d31db1f43078b8b6a43 Mon Sep 17 00:00:00 2001 From: MichalZ <188900745+mrek-msft@users.noreply.github.com> Date: Mon, 20 Apr 2026 16:06:51 +0200 Subject: [PATCH 4/5] Update timeout computation Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/ProcessTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index fc13f08c03cadc..e78aa76ff66302 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -196,10 +196,11 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) const int UnterminatedExitCode = -1; // Process operations timeout cascading: - // WaitInMS * 1: Remote process wait for second signal after unregistering exercised signal handler - // WaitInMS * 2: RemoteExecutor Timeout, collects data and makes dump if not exited gracefully before + // WaitInMS * 1: Remote process wait for first signal that unregisters the exercised signal handler + // WaitInMS * 2: Remote process may additionally wait for a second signal that should terminate it + // WaitInMS * 3: RemoteExecutor timeout, leaving slack for process startup, signal delivery, and teardown - var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false, TimeOut = WaitInMS * 2 }; + var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false, TimeOut = WaitInMS * 3 }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; if (OperatingSystem.IsWindows()) { From ccc23bba2ae2956ebc1ac02033bec25c3e0583a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BD=C5=AFrek?= <188900745+mrek-msft@users.noreply.github.com> Date: Wed, 22 Apr 2026 14:09:51 +0200 Subject: [PATCH 5/5] Fix copilot detected issue with overflowing signed/unsigned return codes on Unix platforom --- src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index fc13f08c03cadc..f48dc940b1a957 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -193,7 +193,7 @@ public void SignalHandler_CanDisposeInHandler(PosixSignal signal) const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created."; const string PosixSignalHandlerStartedMessage = "PosixSignalRegistration handler started."; const string PosixSignalHandlerDisposedMessage = "PosixSignalRegistration disposed."; - const int UnterminatedExitCode = -1; + const int UnterminatedExitCode = 1; // Process operations timeout cascading: // WaitInMS * 1: Remote process wait for second signal after unregistering exercised signal handler