Add CreateNewProcessGroup to ProcessStartInfo (#117105)

* Add CreateNewProcessGroup

* Add RemoteExecutor.IsSupported condition

* Use ManualResetEvent instead of local bool

* Throw instead of asserting on Win pinvoke to get more info about the underlying error

* MRE.WaitOne call should be asserted

* Switch to memberdata to avoid pulluting the console log with skips

* wrap SendSignal in try-finally and kill remote process to avoid hiding SendSignal exception.

* Skip test on Docker

* Guard SkipTestException with  PlatformDetection.IsInContainer

* Add documentation

Author: jozkee

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;
         }
     }
 }

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<string> 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]

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;

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))); }
+        }
     }
 }

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs

@@ -46,5 +46,17 @@ public string Domain
         [CLSCompliant(false)]
         [SupportedOSPlatform("windows")]
         public SecureString? Password { get; set; }
+
+        /// <summary>
+        /// Gets or sets a value indicating whether to start the process in a new process group.
+        /// </summary>
+        /// <value><c>true</c> if the process should be started in a new process group; otherwise, <c>false</c>. The default is <c>false</c>.</value>
+        /// <remarks>
+        /// <para>When a process is created in a new process group, it becomes the root of a new process group.</para>
+        /// <para>An implicit call to <c>SetConsoleCtrlHandler(NULL,TRUE)</c> is made on behalf of the new process, this means that the new process has CTRL+C disabled.</para>
+        /// <para>This property is useful for preventing console control events sent to the child process from affecting the parent process.</para>
+        /// </remarks>
+        [SupportedOSPlatform("windows")]
+        public bool CreateNewProcessGroup { get; set; }
     }
 }

src/libraries/System.Diagnostics.Process/tests/Interop.cs

@@ -72,6 +72,11 @@ public struct SID_AND_ATTRIBUTES
         public int Attributes;
     }
 
+        
+    [DllImport("kernel32.dll", SetLastError = true)]
+    [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);
 

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<PlatformNotSupportedException>(() => info.CreateNewProcessGroup);
+            Assert.Throws<PlatformNotSupportedException>(() => info.CreateNewProcessGroup = true);
+        }
+
         [Theory]
         [InlineData(null)]
         [InlineData("")]

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) { }
     }
 }

src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs

@@ -2,7 +2,11 @@
 // 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 Microsoft.DotNet.XUnitExtensions;
+using Xunit;
 
 namespace System.Diagnostics.Tests
 {
@@ -15,5 +19,41 @@ 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))
+            };
+
+            if (!Interop.GenerateConsoleCtrlEvent(dwCtrlEvent, (uint)processId))
+            {
+                int error = Marshal.GetLastWin32Error();
+                if (error == Interop.Errors.ERROR_INVALID_FUNCTION && PlatformDetection.IsInContainer)
+                {
+                    // 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.");
+                }
+
+                throw new Win32Exception(error);
+            }
+        }
+
+        // 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)
+            {
+                if (!Interop.Kernel32.SetConsoleCtrlHandler(null, false))
+                {
+                    throw new Win32Exception();
+                }
+            }
+        }
     }
 }

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,82 @@ private void AssertNonZeroAllZeroDarwin(long value)
             }
         }
 
+        public static IEnumerable<object[]> SignalTestData()
+        {
+            if (OperatingSystem.IsWindows())
+            {
+                // 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<PosixSignal>())
+                {
+                    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))]
+        [MemberData(nameof(SignalTestData))]
+        public void TestCreateNewProcessGroup_HandlerReceivesExpectedSignal(PosixSignal signal)
+        {
+            const string PosixSignalRegistrationCreatedMessage = "PosixSignalRegistration created...";
+
+            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<PosixSignal>(signalStr);
+                    using ManualResetEvent receivedSignalEvent = new ManualResetEvent(false);
+                    ReEnableCtrlCHandlerIfNeeded(expectedSignal);
+
+                    using PosixSignalRegistration p = PosixSignalRegistration.Create(expectedSignal, (ctx) =>
+                    {
+                        Assert.Equal(expectedSignal, ctx.Signal);
+                        receivedSignalEvent.Set();
+                        ctx.Cancel = true;
+                    });
+
+                    Console.WriteLine(PosixSignalRegistrationCreatedMessage);
+
+                    Assert.True(receivedSignalEvent.WaitOne(WaitInMS));
+
+                    return 0;
+                },
+                arg: $"{signal}",
+                remoteInvokeOptions);
+
+            while (!remoteHandle.Process.StandardOutput.ReadLine().EndsWith(PosixSignalRegistrationCreatedMessage))
+            {
+                Thread.Sleep(20);
+            }
+
+            try
+            {
+                SendSignal(signal, remoteHandle.Process.Id);
+
+                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))]
         [InlineData(-2)]
         [InlineData((long)int.MaxValue + 1)]