From 50ed533375f923fe87ba8a87ef1d364db841b27c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Fri, 27 Jun 2025 21:28:22 +0000 Subject: [PATCH 01/10] Add CreateNewProcessGroup --- .../Advapi32/Interop.ProcessOptions.cs | 1 + .../ref/System.Diagnostics.Process.cs | 2 + .../src/System/Diagnostics/Process.Windows.cs | 1 + .../Diagnostics/ProcessStartInfo.Unix.cs | 7 ++ .../Diagnostics/ProcessStartInfo.Windows.cs | 3 + .../tests/Interop.cs | 5 ++ .../tests/ProcessStartInfoTests.cs | 23 +++++++ .../tests/ProcessTests.Unix.cs | 11 ++++ .../tests/ProcessTests.Windows.cs | 25 ++++++++ .../tests/ProcessTests.cs | 64 +++++++++++++++++++ .../System.Diagnostics.Process.Tests.csproj | 12 +++- 11 files changed, 152 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs index 8ef5167d70edc6..893a0aaed9073c 100644 --- a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs +++ b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs @@ -43,6 +43,7 @@ internal static partial class StartupInfoOptions internal const int STARTF_USESTDHANDLES = 0x00000100; internal const int CREATE_UNICODE_ENVIRONMENT = 0x00000400; internal const int CREATE_NO_WINDOW = 0x08000000; + internal const int CREATE_NEW_PROCESS_GROUP = 0x00000200; } } } 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 d4cf2170b88b83..17366ae9811736 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -219,6 +219,8 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< public System.Collections.ObjectModel.Collection ArgumentList { get { throw null; } } [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string Arguments { get { throw null; } set { } } + [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] + public bool CreateNewProcessGroup { get { throw null; } set { } } public bool CreateNoWindow { get { throw null; } set { } } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] [System.Diagnostics.CodeAnalysis.AllowNullAttribute] diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs index 06a2bd51d6d402..dc05e138a5aec6 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs @@ -505,6 +505,7 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) // set up the creation flags parameter int creationFlags = 0; if (startInfo.CreateNoWindow) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NO_WINDOW; + if (startInfo.CreateNewProcessGroup) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NEW_PROCESS_GROUP; // set up the environment block parameter string? environmentBlock = null; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Unix.cs index 9664815dcde794..196d57bf0b36b9 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Unix.cs @@ -52,5 +52,12 @@ public SecureString? Password get { throw new PlatformNotSupportedException(SR.Format(SR.ProcessStartSingleFeatureNotSupported, nameof(Password))); } set { throw new PlatformNotSupportedException(SR.Format(SR.ProcessStartSingleFeatureNotSupported, nameof(Password))); } } + + [SupportedOSPlatform("windows")] + public bool CreateNewProcessGroup + { + get { throw new PlatformNotSupportedException(SR.Format(SR.ProcessStartSingleFeatureNotSupported, nameof(CreateNewProcessGroup))); } + set { throw new PlatformNotSupportedException(SR.Format(SR.ProcessStartSingleFeatureNotSupported, nameof(CreateNewProcessGroup))); } + } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs index 8869c1809341f3..1c95ccecf83083 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs @@ -46,5 +46,8 @@ public string Domain [CLSCompliant(false)] [SupportedOSPlatform("windows")] public SecureString? Password { get; set; } + + [SupportedOSPlatform("windows")] + public bool CreateNewProcessGroup { get; set; } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/Interop.cs b/src/libraries/System.Diagnostics.Process/tests/Interop.cs index 969995e36d8a33..7911fabb323243 100644 --- a/src/libraries/System.Diagnostics.Process/tests/Interop.cs +++ b/src/libraries/System.Diagnostics.Process/tests/Interop.cs @@ -72,6 +72,11 @@ public struct SID_AND_ATTRIBUTES public int Attributes; } + + [DllImport("kernel32.dll")] + [return: MarshalAs(UnmanagedType.Bool)] + public static extern bool GenerateConsoleCtrlEvent(uint dwCtrlEvent, uint dwProcessGroupId); + [DllImport("kernel32.dll")] public static extern bool GetProcessWorkingSetSizeEx(SafeProcessHandle hProcess, out IntPtr lpMinimumWorkingSetSize, out IntPtr lpMaximumWorkingSetSize, out uint flags); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 728e52f520208f..2e896862d638b5 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -905,6 +905,29 @@ public void TestEnvironmentVariablesPropertyUnix() }); } + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void CreateNewProcessGroup_SetWindows_GetReturnsExpected() + { + ProcessStartInfo psi = new ProcessStartInfo(); + Assert.False(psi.CreateNewProcessGroup); + + psi.CreateNewProcessGroup = true; + Assert.True(psi.CreateNewProcessGroup); + + psi.CreateNewProcessGroup = false; + Assert.False(psi.CreateNewProcessGroup); + } + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix)] + public void CreateNewProcessGroup_GetSetUnix_ThrowsPlatformNotSupportedException() + { + var info = new ProcessStartInfo(); + Assert.Throws(() => info.CreateNewProcessGroup); + Assert.Throws(() => info.CreateNewProcessGroup = true); + } + [Theory] [InlineData(null)] [InlineData("")] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 7f66696c6b2755..47c5ce05870287 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -1047,5 +1047,16 @@ private static string StartAndReadToEnd(string filename, string[] arguments) return process.StandardOutput.ReadToEnd(); } } + + private static void SendSignal(PosixSignal signal, int processId) + { + int result = kill(processId, Interop.Sys.GetPlatformSignalNumber(signal)); + if (result != 0) + { + throw new Win32Exception(Marshal.GetLastWin32Error(), $"Failed to send signal {signal} to process {processId}"); + } + } + + private static unsafe void ReEnableCtrlCHandlerIfNeeded(PosixSignal signal) { } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs index 6fb91c0f4e113e..ebefe0b792ed11 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs @@ -3,6 +3,8 @@ using System; using System.IO; +using System.Runtime.InteropServices; +using Xunit; namespace System.Diagnostics.Tests { @@ -15,5 +17,28 @@ private string WriteScriptFile(string directory, string name, int returnValue) File.WriteAllText(filename, $"exit {returnValue}"); return filename; } + + private static void SendSignal(PosixSignal signal, int processId) + { + uint dwCtrlEvent = signal switch + { + PosixSignal.SIGINT => Interop.Kernel32.CTRL_C_EVENT, + PosixSignal.SIGQUIT => Interop.Kernel32.CTRL_BREAK_EVENT, + _ => throw new ArgumentOutOfRangeException(nameof(signal)) + }; + + Assert.True(Interop.GenerateConsoleCtrlEvent(dwCtrlEvent, (uint)processId)); + } + + // See https://learn.microsoft.com/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw#remarks: + // When a process is created with CREATE_NEW_PROCESS_GROUP specified, an implicit call to SetConsoleCtrlHandler(NULL,TRUE) + // is made on behalf of the new process; this means that the new process has CTRL+C disabled. + private static unsafe void ReEnableCtrlCHandlerIfNeeded(PosixSignal signal) + { + if (signal is PosixSignal.SIGINT) + { + Assert.True(Interop.Kernel32.SetConsoleCtrlHandler(null, false)); + } + } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index a3b2a7a97f0508..a426946b3f5910 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.InteropServices; using System.Security; using System.Text; using System.Threading; @@ -80,6 +81,69 @@ private void AssertNonZeroAllZeroDarwin(long value) } } + [ConditionalTheory] + [InlineData(PosixSignal.SIGTSTP)] + [InlineData(PosixSignal.SIGTTOU)] + [InlineData(PosixSignal.SIGTTIN)] + [InlineData(PosixSignal.SIGWINCH)] + [InlineData(PosixSignal.SIGCONT)] + [InlineData(PosixSignal.SIGCHLD)] + [InlineData(PosixSignal.SIGTERM)] + [InlineData(PosixSignal.SIGQUIT)] + [InlineData(PosixSignal.SIGINT)] + [InlineData(PosixSignal.SIGHUP)] + [InlineData((PosixSignal)3)] // SIGQUIT + [InlineData((PosixSignal)15)] // SIGTERM + public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal signal) + { + const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created..."; + + if (OperatingSystem.IsWindows() && signal is not (PosixSignal.SIGINT or PosixSignal.SIGQUIT)) + { + throw new SkipTestException("GenerateConsoleCtrlEvent does not support sending this signal."); + } + + var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false }; + remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; + if (OperatingSystem.IsWindows()) + { + remoteInvokeOptions.StartInfo.CreateNewProcessGroup = true; + } + + using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke( + (signalStr) => + { + PosixSignal expectedSignal = Enum.Parse(signalStr); + bool receivedSignal = false; + ReEnableCtrlCHandlerIfNeeded(expectedSignal); + + using PosixSignalRegistration p = PosixSignalRegistration.Create(expectedSignal, (ctx) => + { + Assert.Equal(expectedSignal, ctx.Signal); + receivedSignal = true; + ctx.Cancel = true; + }); + + Console.WriteLine(PosixSignalRegistrationCreatedMessage); + + while (!receivedSignal) ; + + return 0; + }, + arg: $"{signal}", + remoteInvokeOptions); + + while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalRegistrationCreatedMessage)) + { + Thread.Sleep(20); + } + + SendSignal(signal, remoteHandle.Process.Id); + + Assert.True(remoteHandle.Process.WaitForExit(WaitInMS)); + Assert.Equal(0, remoteHandle.Process.ExitCode); + } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(-2)] [InlineData((long)int.MaxValue + 1)] diff --git a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index a69d3eb2372059..e06365b1ac1d68 100644 --- a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -44,12 +44,16 @@ + + Link="Common\Interop\Windows\Interop.Libraries.cs" /> + Link="Common\Interop\Windows\Kernel32\Interop.LoadLibrary.cs" /> + @@ -63,6 +67,10 @@ Link="Common\Interop\OSX\Interop.libproc.cs" /> + + From 0ae9e3c72d14f58ade21033cd33cce3b5e9bbe13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Tue, 1 Jul 2025 14:23:25 -0500 Subject: [PATCH 02/10] Add RemoteExecutor.IsSupported condition --- 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 a426946b3f5910..0627f4c16fe9b5 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -81,7 +81,7 @@ private void AssertNonZeroAllZeroDarwin(long value) } } - [ConditionalTheory] + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(PosixSignal.SIGTSTP)] [InlineData(PosixSignal.SIGTTOU)] [InlineData(PosixSignal.SIGTTIN)] From 0f5eb59b3cf0ec6f2df726aa575bf50f9447978b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Mon, 7 Jul 2025 13:59:05 -0500 Subject: [PATCH 03/10] Use ManualResetEvent instead of local bool --- .../System.Diagnostics.Process/tests/ProcessTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 0627f4c16fe9b5..f26c1fbdab803c 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -114,20 +114,20 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal (signalStr) => { PosixSignal expectedSignal = Enum.Parse(signalStr); - bool receivedSignal = false; + using ManualResetEvent receivedSignalEvent = new ManualResetEvent(false); ReEnableCtrlCHandlerIfNeeded(expectedSignal); using PosixSignalRegistration p = PosixSignalRegistration.Create(expectedSignal, (ctx) => { Assert.Equal(expectedSignal, ctx.Signal); - receivedSignal = true; + receivedSignalEvent.Set(); ctx.Cancel = true; }); Console.WriteLine(PosixSignalRegistrationCreatedMessage); - - while (!receivedSignal) ; - + + receivedSignalEvent.WaitOne(WaitInMS); + return 0; }, arg: $"{signal}", From dd9aa05b8699700e83ae7bf62abbd81268dc9c39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Tue, 8 Jul 2025 14:18:35 -0500 Subject: [PATCH 04/10] Throw instead of asserting on Win pinvoke to get more info about the underlying error --- .../System.Diagnostics.Process/tests/Interop.cs | 2 +- .../tests/ProcessTests.Windows.cs | 11 +++++++++-- .../System.Diagnostics.Process/tests/ProcessTests.cs | 4 ++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/Interop.cs b/src/libraries/System.Diagnostics.Process/tests/Interop.cs index 7911fabb323243..6bd2ddebbc8f6a 100644 --- a/src/libraries/System.Diagnostics.Process/tests/Interop.cs +++ b/src/libraries/System.Diagnostics.Process/tests/Interop.cs @@ -73,7 +73,7 @@ public struct SID_AND_ATTRIBUTES } - [DllImport("kernel32.dll")] + [DllImport("kernel32.dll", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] public static extern bool GenerateConsoleCtrlEvent(uint dwCtrlEvent, uint dwProcessGroupId); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs index ebefe0b792ed11..d2503365c0f07d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.ComponentModel; using System.IO; using System.Runtime.InteropServices; using Xunit; @@ -27,7 +28,10 @@ private static void SendSignal(PosixSignal signal, int processId) _ => throw new ArgumentOutOfRangeException(nameof(signal)) }; - Assert.True(Interop.GenerateConsoleCtrlEvent(dwCtrlEvent, (uint)processId)); + if (!Interop.GenerateConsoleCtrlEvent(dwCtrlEvent, (uint)processId)) + { + throw new Win32Exception(); + } } // See https://learn.microsoft.com/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw#remarks: @@ -37,7 +41,10 @@ private static unsafe void ReEnableCtrlCHandlerIfNeeded(PosixSignal signal) { if (signal is PosixSignal.SIGINT) { - Assert.True(Interop.Kernel32.SetConsoleCtrlHandler(null, false)); + if (!Interop.Kernel32.SetConsoleCtrlHandler(null, false)) + { + throw new Win32Exception(); + } } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index f26c1fbdab803c..d95133850dafd4 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -125,9 +125,9 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal }); Console.WriteLine(PosixSignalRegistrationCreatedMessage); - + receivedSignalEvent.WaitOne(WaitInMS); - + return 0; }, arg: $"{signal}", From e6fe7d8f7ea827fae6e358a530c0845e246d21ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Tue, 8 Jul 2025 14:20:07 -0500 Subject: [PATCH 05/10] MRE.WaitOne call should be asserted --- 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 d95133850dafd4..3cb67888963050 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -126,7 +126,7 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal Console.WriteLine(PosixSignalRegistrationCreatedMessage); - receivedSignalEvent.WaitOne(WaitInMS); + Assert.True(receivedSignalEvent.WaitOne(WaitInMS)); return 0; }, From ac2bf2343de4f2f8f89f0b5fc12a481051e0db4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Tue, 8 Jul 2025 14:36:18 -0500 Subject: [PATCH 06/10] Switch to memberdata to avoid pulluting the console log with skips --- .../tests/ProcessTests.cs | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 3cb67888963050..9e4eba6bdd035d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -81,28 +81,34 @@ private void AssertNonZeroAllZeroDarwin(long value) } } + public static IEnumerable SignalTestData() + { + if (OperatingSystem.IsWindows()) + { + // .NET already maps POSIX signals to Windows console events + // https://github.com/dotnet/runtime/blob/d221687c3724d26d653d022f4b254bc1d7eb1a6b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs#L17-L18 + // GenerateConsoleCtrlEvent only supports sending CTRL_C_EVENT and CTRL_BREAK_EVENT + yield return new object[] { PosixSignal.SIGINT }; + yield return new object[] { PosixSignal.SIGQUIT }; + } + else + { + foreach (PosixSignal signal in Enum.GetValues()) + { + yield return new object[] { signal }; + } + // Test a few raw signals. + yield return new object[] { (PosixSignal)3 }; // SIGQUIT + yield return new object[] { (PosixSignal)15 }; // SIGTERM + } + } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(PosixSignal.SIGTSTP)] - [InlineData(PosixSignal.SIGTTOU)] - [InlineData(PosixSignal.SIGTTIN)] - [InlineData(PosixSignal.SIGWINCH)] - [InlineData(PosixSignal.SIGCONT)] - [InlineData(PosixSignal.SIGCHLD)] - [InlineData(PosixSignal.SIGTERM)] - [InlineData(PosixSignal.SIGQUIT)] - [InlineData(PosixSignal.SIGINT)] - [InlineData(PosixSignal.SIGHUP)] - [InlineData((PosixSignal)3)] // SIGQUIT - [InlineData((PosixSignal)15)] // SIGTERM + [MemberData(nameof(SignalTestData))] public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal signal) { const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created..."; - if (OperatingSystem.IsWindows() && signal is not (PosixSignal.SIGINT or PosixSignal.SIGQUIT)) - { - throw new SkipTestException("GenerateConsoleCtrlEvent does not support sending this signal."); - } - var remoteInvokeOptions = new RemoteInvokeOptions { CheckExitCode = false }; remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; if (OperatingSystem.IsWindows()) From 4af7d7df9d521fc33537c1e96311bbc4c5ad0d18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Tue, 8 Jul 2025 23:18:20 -0500 Subject: [PATCH 07/10] wrap SendSignal in try-finally and kill remote process to avoid hiding SendSignal exception. --- .../tests/ProcessTests.cs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 9e4eba6bdd035d..59f010f1941792 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -85,8 +85,6 @@ public static IEnumerable SignalTestData() { if (OperatingSystem.IsWindows()) { - // .NET already maps POSIX signals to Windows console events - // https://github.com/dotnet/runtime/blob/d221687c3724d26d653d022f4b254bc1d7eb1a6b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Windows.cs#L17-L18 // GenerateConsoleCtrlEvent only supports sending CTRL_C_EVENT and CTRL_BREAK_EVENT yield return new object[] { PosixSignal.SIGINT }; yield return new object[] { PosixSignal.SIGQUIT }; @@ -144,10 +142,19 @@ public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal Thread.Sleep(20); } - SendSignal(signal, remoteHandle.Process.Id); + try + { + SendSignal(signal, remoteHandle.Process.Id); - Assert.True(remoteHandle.Process.WaitForExit(WaitInMS)); - Assert.Equal(0, remoteHandle.Process.ExitCode); + Assert.True(remoteHandle.Process.WaitForExit(WaitInMS)); + Assert.Equal(0, remoteHandle.Process.ExitCode); + } + finally + { + // If sending the signal fails, we want to kill the process ASAP + // to prevent RemoteExecutor's timeout from hiding it. + remoteHandle.Process.Kill(); + } } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] From e39df301379c2d32980692e5e8a779ea1bae8bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Wed, 9 Jul 2025 11:14:37 -0500 Subject: [PATCH 08/10] Skip test on Docker --- .../tests/ProcessTests.Windows.cs | 10 +++++++++- .../tests/System.Diagnostics.Process.Tests.csproj | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs index d2503365c0f07d..eb3a9cb797fe55 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs @@ -5,6 +5,7 @@ using System.ComponentModel; using System.IO; using System.Runtime.InteropServices; +using Microsoft.DotNet.XUnitExtensions; using Xunit; namespace System.Diagnostics.Tests @@ -30,7 +31,14 @@ private static void SendSignal(PosixSignal signal, int processId) if (!Interop.GenerateConsoleCtrlEvent(dwCtrlEvent, (uint)processId)) { - throw new Win32Exception(); + int error = Marshal.GetLastWin32Error(); + if (error == Interop.Errors.ERROR_INVALID_FUNCTION) + { + // This is the case for Docker in CI. + throw new SkipTestException($"GenerateConsoleCtrlEvent failed with ERROR_INVALID_FUNCTION. The process is not a console process or does not have a console."); + } + + throw new Win32Exception(error); } } diff --git a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index e06365b1ac1d68..718bddd017ea89 100644 --- a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -54,6 +54,8 @@ Link="Common\Interop\Windows\Kernel32\Interop.FreeLibrary.cs" /> + From 8305863c79dae9d0a48261853aa33d4c688bec76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Wed, 9 Jul 2025 14:18:45 -0500 Subject: [PATCH 09/10] Guard SkipTestException with PlatformDetection.IsInContainer --- .../System.Diagnostics.Process/tests/ProcessTests.Windows.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs index eb3a9cb797fe55..7850ca8afcbd7d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs @@ -32,9 +32,9 @@ private static void SendSignal(PosixSignal signal, int processId) if (!Interop.GenerateConsoleCtrlEvent(dwCtrlEvent, (uint)processId)) { int error = Marshal.GetLastWin32Error(); - if (error == Interop.Errors.ERROR_INVALID_FUNCTION) + if (error == Interop.Errors.ERROR_INVALID_FUNCTION && PlatformDetection.IsInContainer) { - // This is the case for Docker in CI. + // Docker in CI runs without a console attached. throw new SkipTestException($"GenerateConsoleCtrlEvent failed with ERROR_INVALID_FUNCTION. The process is not a console process or does not have a console."); } From d6664bbe7d01bd84a32542afba773d567ad6e1c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Wed, 9 Jul 2025 14:50:52 -0500 Subject: [PATCH 10/10] Add documentation --- .../src/System/Diagnostics/ProcessStartInfo.Windows.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs index 1c95ccecf83083..6217aeb9c5a5b3 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs @@ -47,6 +47,15 @@ public string Domain [SupportedOSPlatform("windows")] public SecureString? Password { get; set; } + /// + /// Gets or sets a value indicating whether to start the process in a new process group. + /// + /// true if the process should be started in a new process group; otherwise, false. The default is false. + /// + /// When a process is created in a new process group, it becomes the root of a new process group. + /// An implicit call to SetConsoleCtrlHandler(NULL,TRUE) is made on behalf of the new process, this means that the new process has CTRL+C disabled. + /// This property is useful for preventing console control events sent to the child process from affecting the parent process. + /// [SupportedOSPlatform("windows")] public bool CreateNewProcessGroup { get; set; } }