From 4f3f95074cdd20f1106e8bf562d2dfbe538ffeda Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 13:33:47 +0000 Subject: [PATCH 01/33] Initial plan From 92de2998c9470c6d5da14ea519f77dd5fe438465 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 13:42:28 +0000 Subject: [PATCH 02/33] Add ProcessStartOptions class with path resolution and tests Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Kernel32/Interop.GetWindowsDirectoryW.cs | 13 + .../ref/System.Diagnostics.Process.cs | 11 + .../src/System.Diagnostics.Process.csproj | 6 + .../System/Diagnostics/ProcessStartOptions.cs | 348 ++++++++++++++++++ .../tests/ProcessStartOptionsTests.Unix.cs | 203 ++++++++++ .../tests/ProcessStartOptionsTests.Windows.cs | 200 ++++++++++ .../tests/ProcessStartOptionsTests.cs | 224 +++++++++++ .../System.Diagnostics.Process.Tests.csproj | 3 + 8 files changed, 1008 insertions(+) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs new file mode 100644 index 00000000000000..05710a47a019f9 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + internal static partial uint GetWindowsDirectoryW(ref char lpBuffer, uint uSize); + } +} 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 17366ae9811736..c6b6565016ea69 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -261,6 +261,17 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string WorkingDirectory { get { throw null; } set { } } } + public sealed partial class ProcessStartOptions + { + public ProcessStartOptions(string fileName) { } + public System.Collections.Generic.IList Arguments { get { throw null; } set { } } + public bool CreateNewProcessGroup { get { throw null; } set { } } + public System.Collections.Generic.IDictionary Environment { get { throw null; } } + public string FileName { get { throw null; } } + public System.Collections.Generic.IList InheritedHandles { get { throw null; } set { } } + public bool KillOnParentExit { get { throw null; } set { } } + public string? WorkingDirectory { get { throw null; } set { } } + } [System.ComponentModel.DesignerAttribute("System.Diagnostics.Design.ProcessThreadDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class ProcessThread : System.ComponentModel.Component { diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 986ee67013d3d7..a674056838985d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -12,6 +12,7 @@ $([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) SR.Process_PlatformNotSupported true + $(DefineConstants);WINDOWS @@ -26,6 +27,7 @@ + @@ -136,6 +138,10 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetProcessPriorityBoost.cs" /> + + + /// Specifies options for starting a new process. + /// + public sealed class ProcessStartOptions + { + private readonly string _fileName; + private IList? _arguments; + private DictionaryWrapper? _environment; + private IList? _inheritedHandles; + + /// + /// Gets the application to start. + /// + public string FileName => _fileName; + + /// + /// Gets or sets the command-line arguments to pass to the application. + /// + public IList Arguments + { + get => _arguments ??= new List(); + set => _arguments = value; + } + + /// + /// Gets the environment variables that apply to this process and its child processes. + /// + /// + /// By default, the environment is a copy of the current process environment. + /// + public IDictionary Environment + { + get + { + if (_environment == null) + { + IDictionary envVars = System.Environment.GetEnvironmentVariables(); + + _environment = new DictionaryWrapper(new Dictionary( + envVars.Count, + OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal)); + + // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations. + IDictionaryEnumerator e = envVars.GetEnumerator(); + Debug.Assert(!(e is IDisposable), "Environment.GetEnvironmentVariables should not be IDisposable."); + while (e.MoveNext()) + { + DictionaryEntry entry = e.Entry; + _environment.Add((string)entry.Key, (string?)entry.Value); + } + } + return _environment; + } + } + + /// + /// Gets or sets the working directory for the process to be started. + /// + public string? WorkingDirectory { get; set; } + + /// + /// Gets a list of handles that will be inherited by the child process. + /// + /// + /// + /// Handles do not need to have inheritance enabled beforehand. + /// They are also not duplicated, just added as-is to the child process + /// so the exact same handle values can be used in the child process. + /// + /// + /// On Windows, the implementation will automatically enable inheritance on any handle added to this list + /// by modifying the handle's flags using SetHandleInformation. + /// + /// + /// On Unix, the implementation will modify the copy of every handle in the child process + /// by removing FD_CLOEXEC flag. It happens after the fork and before the exec, so it does not affect parent process. + /// + /// + public IList InheritedHandles + { + get => _inheritedHandles ??= new List(); + set => _inheritedHandles = value; + } + + /// + /// Gets or sets a value indicating whether the child process should be terminated when the parent process exits. + /// + public bool KillOnParentExit { get; set; } + + /// + /// Gets or sets a value indicating whether to create the process in a new process group. + /// + /// + /// + /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) + /// on Windows and provides process group isolation on all platforms. + /// + /// + /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's + /// process group, which can be useful for background processes that should continue running independently. + /// + /// + public bool CreateNewProcessGroup { get; set; } + + /// + /// Initializes a new instance of the class. + /// + /// The application to start. + /// Thrown when is null or empty. + /// Thrown when cannot be resolved to an existing file. + public ProcessStartOptions(string fileName) + { + ArgumentException.ThrowIfNullOrEmpty(fileName); + + _fileName = ResolvePath(fileName); + } + + /// Resolves a path to the filename. + /// The filename. + /// The resolved path. + /// Thrown when cannot be resolved to an existing file. + internal static string ResolvePath(string filename) + { + // If the filename is a complete path, use it, regardless of whether it exists. + if (Path.IsPathRooted(filename)) + { + // In this case, it doesn't matter whether the file exists or not; + // it's what the caller asked for, so it's what they'll get + return filename; + } + +#if WINDOWS + // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw + // "If the file name does not contain an extension, .exe is appended. + // Therefore, if the file name extension is .com, this parameter must include the .com extension. + // If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended." + + // HasExtension returns false for trailing dot, so we need to check that separately + if (filename[filename.Length - 1] != '.' && !Path.HasExtension(filename)) + { + filename += ".exe"; + } +#endif + + // Then check the executable's directory + string? path = Environment.ProcessPath; + if (path != null) + { + try + { + path = Path.Combine(Path.GetDirectoryName(path)!, filename); + if (File.Exists(path)) + { + return path; + } + } + catch (ArgumentException) { } // ignore any errors in data that may come from the exe path + } + + // Then check the current directory + path = Path.Combine(Directory.GetCurrentDirectory(), filename); + if (File.Exists(path)) + { + return path; + } + +#if WINDOWS + // Windows-specific search locations (from CreateProcessW documentation) + + // Check the 32-bit Windows system directory (It can't change over app lifetime) + path = GetSystemDirectory(); + if (path != null) + { + path = Path.Combine(path, filename); + if (File.Exists(path)) + { + return path; + } + } + + // Check the Windows directory + path = GetWindowsDirectory(); + if (path != null) + { + // Check the 16-bit Windows system directory (System subdirectory of Windows directory) + string systemPath = Path.Combine(path, "System", filename); + if (File.Exists(systemPath)) + { + return systemPath; + } + + // Check the Windows directory itself + path = Path.Combine(path, filename); + if (File.Exists(path)) + { + return path; + } + } +#endif + + // Then check each directory listed in the PATH environment variables + return FindProgramInPath(filename); + } + + /// + /// Gets the path to the program by searching in PATH environment variable. + /// + /// The program name. + /// The full path to the program. + /// Thrown when the program cannot be found in PATH. + private static string FindProgramInPath(string program) + { + string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); + if (pathEnvVar != null) + { +#if WINDOWS + char pathSeparator = ';'; +#else + char pathSeparator = ':'; +#endif + var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); + while (pathParser.MoveNext()) + { + string subPath = pathParser.ExtractCurrent(); + string path = Path.Combine(subPath, program); + if (IsExecutableFile(path)) + { + return path; + } + } + } + + throw new FileNotFoundException("Could not resolve the file.", program); + } + + private static bool IsExecutableFile(string path) + { +#if WINDOWS + return File.Exists(path); +#else + return IsExecutable(path); +#endif + } + +#if !WINDOWS + private static bool IsExecutable(string fullPath) + { + Interop.Sys.FileStatus fileinfo; + + if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) + { + return false; + } + + // Check if the path is a directory. + if ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) + { + return false; + } + + const UnixFileMode AllExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; + + UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; + + // Avoid checking user/group when permission. + if (permissions == AllExecute) + { + return true; + } + else if (permissions == 0) + { + return false; + } + + uint euid = Interop.Sys.GetEUid(); + + if (euid == 0) + { + return true; // We're root. + } + + if (euid == fileinfo.Uid) + { + // We own the file. + return (permissions & UnixFileMode.UserExecute) != 0; + } + + bool groupCanExecute = (permissions & UnixFileMode.GroupExecute) != 0; + bool otherCanExecute = (permissions & UnixFileMode.OtherExecute) != 0; + + // Avoid group check when group and other have same permissions. + if (groupCanExecute == otherCanExecute) + { + return groupCanExecute; + } + + if (Interop.Sys.IsMemberOfGroup(fileinfo.Gid)) + { + return groupCanExecute; + } + else + { + return otherCanExecute; + } + } +#endif + +#if WINDOWS + private static string? s_cachedSystemDirectory; + + private static string? GetSystemDirectory() + { + if (s_cachedSystemDirectory == null) + { + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetSystemDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + s_cachedSystemDirectory = new string(buffer.Slice(0, (int)length)); + } + } + return s_cachedSystemDirectory; + } + + private static string? GetWindowsDirectory() + { + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + return new string(buffer.Slice(0, (int)length)); + } + return null; + } +#endif + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs new file mode 100644 index 00000000000000..2cb8c3cc5f5488 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -0,0 +1,203 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using Xunit; + +namespace System.Diagnostics.Tests +{ + [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] + public class ProcessStartOptionsTests_Unix + { + [Fact] + public void TestResolvePath_FindsInPath() + { + // sh should be findable in PATH on all Unix systems + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.True(File.Exists(options.FileName)); + Assert.Contains("sh", options.FileName); + } + + [Fact] + public void TestResolvePath_DoesNotAddExeExtension() + { + // On Unix, no .exe extension should be added + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.False(options.FileName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public void TestResolvePath_UsesCurrentDirectory() + { + string tempDir = Path.GetTempPath(); + string fileName = "testscript.sh"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Make it executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [Fact] + public void TestResolvePath_PathSeparatorIsColon() + { + // Create a temp directory and file + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + string fileName = "testscript"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Make it executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + // Add temp directory to PATH using colon separator + string oldPath = Environment.GetEnvironmentVariable("PATH"); + try + { + Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Environment.SetEnvironmentVariable("PATH", oldPath); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, recursive: true); + } + } + } + + [Fact] + public void TestResolvePath_ChecksExecutablePermissions() + { + // Create a file without execute permissions + string tempDir = Path.GetTempPath(); + string fileName = "nonexecutable.sh"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Explicitly make it non-executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + // Should throw because file is not executable + Assert.Throws(() => new ProcessStartOptions(fileName)); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [Fact] + public void TestResolvePath_AbsolutePathIsNotModified() + { + string tempFile = Path.GetTempFileName(); + try + { + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + if (File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } + + [Fact] + public void TestResolvePath_FindsCommonUtilities() + { + // Test common Unix utilities + string[] commonUtils = { "ls", "cat", "echo", "sh" }; + + foreach (string util in commonUtils) + { + ProcessStartOptions options = new ProcessStartOptions(util); + Assert.True(File.Exists(options.FileName), $"{util} should be found and exist"); + Assert.Contains(util, options.FileName); + } + } + + [Fact] + public void TestResolvePath_RejectsDirectories() + { + // Create a directory with executable permissions + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + + try + { + // Try to use the directory name as a command + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(Path.GetTempPath()); + Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir); + } + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs new file mode 100644 index 00000000000000..287cb573fedf16 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -0,0 +1,200 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using Microsoft.DotNet.XUnitExtensions; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessStartOptionsTests_Windows + { + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_AddsExeExtension() + { + // Test that .exe is appended when no extension is provided + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.True(File.Exists(options.FileName)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() + { + // "If the file name ends in a period (.) with no extension, .exe is not appended." + // This should fail since "notepad." won't exist + Assert.Throws(() => new ProcessStartOptions("notepad.")); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_PreservesComExtension() + { + // The .com extension should be preserved + string fileName = "test.com"; + string tempDir = Path.GetTempPath(); + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInSystemDirectory() + { + // cmd.exe should be found in system directory + ProcessStartOptions options = new ProcessStartOptions("cmd"); + Assert.True(File.Exists(options.FileName)); + Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInWindowsDirectory() + { + // Some utilities are in Windows directory + // We'll test with a file that's commonly in Windows directory + // Note: This might not exist on all systems, so we make it conditional + try + { + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.True(File.Exists(options.FileName)); + } + catch (FileNotFoundException) + { + // Skip if notepad is not found - it's not critical for this test + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInPath() + { + // powershell.exe should be findable in PATH on most Windows systems + try + { + ProcessStartOptions options = new ProcessStartOptions("powershell"); + Assert.True(File.Exists(options.FileName)); + Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); + } + catch (FileNotFoundException) + { + // Skip if PowerShell is not found - it might not be in PATH + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_UsesCurrentDirectory() + { + string tempDir = Path.GetTempPath(); + string fileName = "testapp.exe"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_PathSeparatorIsSemicolon() + { + // Create a temp directory and file + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + string fileName = "testexe.exe"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Add temp directory to PATH using semicolon separator + string oldPath = Environment.GetEnvironmentVariable("PATH"); + try + { + Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); + ProcessStartOptions options = new ProcessStartOptions("testexe"); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Environment.SetEnvironmentVariable("PATH", oldPath); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, recursive: true); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_AbsolutePathIsNotModified() + { + string tempFile = Path.GetTempFileName(); + try + { + // Rename to remove extension to test that .exe is not added for absolute paths + string noExtFile = Path.ChangeExtension(tempFile, null); + File.Move(tempFile, noExtFile); + tempFile = noExtFile; + + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + if (File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs new file mode 100644 index 00000000000000..cf80a3cd7181ce --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -0,0 +1,224 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.IO; +using System.Runtime.InteropServices; +using Microsoft.DotNet.RemoteExecutor; +using Microsoft.DotNet.XUnitExtensions; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessStartOptionsTests : ProcessTestBase + { + [Fact] + public void TestConstructor_NullFileName_Throws() + { + Assert.Throws(() => new ProcessStartOptions(null)); + } + + [Fact] + public void TestConstructor_EmptyFileName_Throws() + { + Assert.Throws(() => new ProcessStartOptions(string.Empty)); + } + + [Fact] + public void TestConstructor_NonExistentFile_Throws() + { + string nonExistentFile = "ThisFileDoesNotExist_" + Guid.NewGuid().ToString(); + Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestConstructor_ResolvesCmdOnWindows() + { + if (!OperatingSystem.IsWindows()) + { + return; + } + + ProcessStartOptions options = new ProcessStartOptions("cmd"); + Assert.Contains("cmd.exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.True(File.Exists(options.FileName)); + } + + [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] + [Fact] + public void TestConstructor_ResolvesShOnUnix() + { + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.Contains("sh", options.FileName); + Assert.True(File.Exists(options.FileName)); + } + + [Fact] + public void TestConstructor_WithAbsolutePath() + { + string tempFile = Path.GetTempFileName(); + try + { + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + File.Delete(tempFile); + } + } + + [Fact] + public void TestArguments_LazyInitialization() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IList args = options.Arguments; + Assert.NotNull(args); + Assert.Empty(args); + } + + [Fact] + public void TestArguments_CanAddAndModify() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.Arguments.Add("arg1"); + options.Arguments.Add("arg2"); + Assert.Equal(2, options.Arguments.Count); + Assert.Equal("arg1", options.Arguments[0]); + Assert.Equal("arg2", options.Arguments[1]); + + options.Arguments = new List { "newArg" }; + Assert.Single(options.Arguments); + Assert.Equal("newArg", options.Arguments[0]); + } + + [Fact] + public void TestEnvironment_LazyInitialization() + { + if (PlatformDetection.IsiOS || PlatformDetection.IstvOS || PlatformDetection.IsMacCatalyst) + { + // Whole list of environment variables can no longer be accessed on non-OSX apple platforms + return; + } + + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + Assert.NotNull(env); + Assert.NotEmpty(env); + } + + [Fact] + public void TestEnvironment_CanAddAndModify() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + + int originalCount = env.Count; + env["TestKey1"] = "TestValue1"; + env["TestKey2"] = "TestValue2"; + Assert.Equal(originalCount + 2, env.Count); + Assert.Equal("TestValue1", env["TestKey1"]); + Assert.Equal("TestValue2", env["TestKey2"]); + + env.Remove("TestKey1"); + Assert.Equal(originalCount + 1, env.Count); + Assert.False(env.ContainsKey("TestKey1")); + } + + [Fact] + public void TestEnvironment_CaseInsensitivityOnWindows() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + + env["TestKey"] = "TestValue"; + + if (OperatingSystem.IsWindows()) + { + Assert.True(env.ContainsKey("testkey")); + Assert.Equal("TestValue", env["TESTKEY"]); + } + else + { + Assert.False(env.ContainsKey("testkey")); + } + } + + [Fact] + public void TestInheritedHandles_LazyInitialization() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IList handles = options.InheritedHandles; + Assert.NotNull(handles); + Assert.Empty(handles); + } + + [Fact] + public void TestInheritedHandles_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + List newHandles = new List(); + options.InheritedHandles = newHandles; + Assert.Same(newHandles, options.InheritedHandles); + } + + [Fact] + public void TestWorkingDirectory_DefaultIsNull() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.Null(options.WorkingDirectory); + } + + [Fact] + public void TestWorkingDirectory_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + string tempDir = Path.GetTempPath(); + options.WorkingDirectory = tempDir; + Assert.Equal(tempDir, options.WorkingDirectory); + } + + [Fact] + public void TestKillOnParentExit_DefaultIsFalse() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.False(options.KillOnParentExit); + } + + [Fact] + public void TestKillOnParentExit_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.KillOnParentExit = true; + Assert.True(options.KillOnParentExit); + } + + [Fact] + public void TestCreateNewProcessGroup_DefaultIsFalse() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.False(options.CreateNewProcessGroup); + } + + [Fact] + public void TestCreateNewProcessGroup_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.CreateNewProcessGroup = true; + Assert.True(options.CreateNewProcessGroup); + } + + private string GetCurrentProcessName() + { + // Get a valid executable path for testing + if (OperatingSystem.IsWindows()) + { + return Path.Combine(Environment.SystemDirectory, "cmd.exe"); + } + else + { + return "/bin/sh"; + } + } + } +} 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 93cb8d34b091e1..63523ef48c86fb 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 @@ -28,6 +28,7 @@ + @@ -40,6 +41,7 @@ + + From 0c9d3e24d4f6e8107da0ef0c4af7c1703c16b40c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:07:48 +0000 Subject: [PATCH 03/33] Fix build errors and test failures in ProcessStartOptions Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System.Diagnostics.Process.csproj | 4 ++-- .../System/Diagnostics/ProcessStartOptions.cs | 17 +++++++++++++---- .../tests/ProcessStartOptionsTests.cs | 2 +- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index a674056838985d..78eee4897b0b89 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -49,6 +49,8 @@ Link="Common\System\Text\ValueStringBuilder.cs" /> + @@ -241,8 +243,6 @@ - InheritedHandles /// /// /// - /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) + /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) /// on Windows and provides process group isolation on all platforms. /// /// - /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's + /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's /// process group, which can be useful for background processes that should continue running independently. /// /// @@ -154,13 +155,17 @@ internal static string ResolvePath(string filename) #endif // Then check the executable's directory - string? path = Environment.ProcessPath; + string? path = System.Environment.ProcessPath; if (path != null) { try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -170,7 +175,11 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -221,7 +230,7 @@ internal static string ResolvePath(string filename) /// Thrown when the program cannot be found in PATH. private static string FindProgramInPath(string program) { - string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); + string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { #if WINDOWS diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index cf80a3cd7181ce..da8c649e8037a7 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -15,7 +15,7 @@ public class ProcessStartOptionsTests : ProcessTestBase [Fact] public void TestConstructor_NullFileName_Throws() { - Assert.Throws(() => new ProcessStartOptions(null)); + Assert.Throws(() => new ProcessStartOptions(null)); } [Fact] From 794a8a206dffbbbf4bb406b95b3f328798616647 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:09:54 +0000 Subject: [PATCH 04/33] Improve code comments based on review feedback Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index f96cf7d9d8abf6..ea6e9799952ab9 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -187,7 +187,7 @@ internal static string ResolvePath(string filename) #if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) - // Check the 32-bit Windows system directory (It can't change over app lifetime) + // Check the system directory (e.g., System32) path = GetSystemDirectory(); if (path != null) { @@ -202,7 +202,7 @@ internal static string ResolvePath(string filename) path = GetWindowsDirectory(); if (path != null) { - // Check the 16-bit Windows system directory (System subdirectory of Windows directory) + // Check the legacy System subdirectory of Windows directory (for compatibility) string systemPath = Path.Combine(path, "System", filename); if (File.Exists(systemPath)) { @@ -282,7 +282,7 @@ private static bool IsExecutable(string fullPath) UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; - // Avoid checking user/group when permission. + // Avoid checking user/group when all execute permissions are set. if (permissions == AllExecute) { return true; From a1b34263e30cf4ce70534bf2e112972b0f0cf880 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:11:44 +0000 Subject: [PATCH 05/33] Fix exception documentation and add caching comment Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index ea6e9799952ab9..16a1cf66178a00 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -118,7 +118,8 @@ public IList InheritedHandles /// Initializes a new instance of the class. /// /// The application to start. - /// Thrown when is null or empty. + /// Thrown when is null. + /// Thrown when is empty. /// Thrown when cannot be resolved to an existing file. public ProcessStartOptions(string fileName) { @@ -344,6 +345,7 @@ private static bool IsExecutable(string fullPath) private static string? GetWindowsDirectory() { + // Don't cache Windows directory as it's user-specific and can change during app lifetime Span buffer = stackalloc char[260]; // MAX_PATH uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); if (length > 0 && length < buffer.Length) From 10ff95ae2a5112f28a0c43589abb4dd0b95f7c41 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 16:09:04 +0000 Subject: [PATCH 06/33] Address PR feedback: use Process.ResolvePath, fix tests, add null checks Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Unix.cs | 6 +- .../System/Diagnostics/ProcessStartOptions.cs | 123 +++++------------- .../tests/ProcessStartOptionsTests.Unix.cs | 19 +-- .../tests/ProcessStartOptionsTests.cs | 15 +-- 4 files changed, 43 insertions(+), 120 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 2dde9eb778cdc9..b2fe96562dfcd9 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -685,7 +685,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) /// Resolves a path to the filename passed to ProcessStartInfo. /// The filename. /// The resolved path. It can return null in case of URLs. - private static string? ResolvePath(string filename) + internal static string? ResolvePath(string filename) { // Follow the same resolution that Windows uses with CreateProcess: // 1. First try the exact path provided @@ -734,7 +734,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) /// /// /// - private static string? FindProgramInPath(string program) + internal static string? FindProgramInPath(string program) { string path; string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); @@ -754,7 +754,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) return null; } - private static bool IsExecutable(string fullPath) + internal static bool IsExecutable(string fullPath) { Interop.Sys.FileStatus fileinfo; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 16a1cf66178a00..b70b042bc96ca5 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,7 +31,11 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set => _arguments = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _arguments = value; + } } /// @@ -91,7 +95,11 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set => _inheritedHandles = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _inheritedHandles = value; + } } /// @@ -134,6 +142,21 @@ public ProcessStartOptions(string fileName) /// Thrown when cannot be resolved to an existing file. internal static string ResolvePath(string filename) { +#if WINDOWS + return ResolvePathWindows(filename); +#else + string? resolved = Process.ResolvePath(filename); + if (resolved == null) + { + throw new FileNotFoundException("Could not resolve the file.", filename); + } + return resolved; +#endif + } + +#if WINDOWS + private static string ResolvePathWindows(string filename) + { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) { @@ -142,7 +165,6 @@ internal static string ResolvePath(string filename) return filename; } -#if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. // Therefore, if the file name extension is .com, this parameter must include the .com extension. @@ -153,7 +175,6 @@ internal static string ResolvePath(string filename) { filename += ".exe"; } -#endif // Then check the executable's directory string? path = System.Environment.ProcessPath; @@ -162,11 +183,7 @@ internal static string ResolvePath(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -176,16 +193,11 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } -#if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) @@ -217,10 +229,9 @@ internal static string ResolvePath(string filename) return path; } } -#endif // Then check each directory listed in the PATH environment variables - return FindProgramInPath(filename); + return FindProgramInPathWindows(filename); } /// @@ -229,22 +240,17 @@ internal static string ResolvePath(string filename) /// The program name. /// The full path to the program. /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPath(string program) + private static string FindProgramInPathWindows(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { -#if WINDOWS - char pathSeparator = ';'; -#else - char pathSeparator = ':'; -#endif - var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); + var pathParser = new StringParser(pathEnvVar, ';', skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (IsExecutableFile(path)) + if (File.Exists(path)) { return path; } @@ -253,77 +259,6 @@ private static string FindProgramInPath(string program) throw new FileNotFoundException("Could not resolve the file.", program); } - - private static bool IsExecutableFile(string path) - { -#if WINDOWS - return File.Exists(path); -#else - return IsExecutable(path); -#endif - } - -#if !WINDOWS - private static bool IsExecutable(string fullPath) - { - Interop.Sys.FileStatus fileinfo; - - if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) - { - return false; - } - - // Check if the path is a directory. - if ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) - { - return false; - } - - const UnixFileMode AllExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; - - UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; - - // Avoid checking user/group when all execute permissions are set. - if (permissions == AllExecute) - { - return true; - } - else if (permissions == 0) - { - return false; - } - - uint euid = Interop.Sys.GetEUid(); - - if (euid == 0) - { - return true; // We're root. - } - - if (euid == fileinfo.Uid) - { - // We own the file. - return (permissions & UnixFileMode.UserExecute) != 0; - } - - bool groupCanExecute = (permissions & UnixFileMode.GroupExecute) != 0; - bool otherCanExecute = (permissions & UnixFileMode.OtherExecute) != 0; - - // Avoid group check when group and other have same permissions. - if (groupCanExecute == otherCanExecute) - { - return groupCanExecute; - } - - if (Interop.Sys.IsMemberOfGroup(fileinfo.Gid)) - { - return groupCanExecute; - } - else - { - return otherCanExecute; - } - } #endif #if WINDOWS diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 2cb8c3cc5f5488..79ba8ced885c37 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -15,7 +15,7 @@ public void TestResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new ProcessStartOptions("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Contains("sh", options.FileName); + Assert.EndsWith("sh", options.FileName); } [Fact] @@ -33,27 +33,20 @@ public void TestResolvePath_UsesCurrentDirectory() string fileName = "testscript.sh"; string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "#!/bin/sh\necho test"); // Make it executable File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index da8c649e8037a7..4bd075a8227948 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -31,16 +31,11 @@ public void TestConstructor_NonExistentFile_Throws() Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void TestConstructor_ResolvesCmdOnWindows() { - if (!OperatingSystem.IsWindows()) - { - return; - } - ProcessStartOptions options = new ProcessStartOptions("cmd"); - Assert.Contains("cmd.exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.EndsWith("cmd.exe", options.FileName); Assert.True(File.Exists(options.FileName)); } @@ -69,7 +64,7 @@ public void TestConstructor_WithAbsolutePath() } [Fact] - public void TestArguments_LazyInitialization() + public void TestArguments_DefaultIsEmpty() { ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); IList args = options.Arguments; @@ -145,7 +140,7 @@ public void TestEnvironment_CaseInsensitivityOnWindows() } [Fact] - public void TestInheritedHandles_LazyInitialization() + public void TestInheritedHandles_DefaultIsEmpty() { ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); IList handles = options.InheritedHandles; @@ -217,7 +212,7 @@ private string GetCurrentProcessName() } else { - return "/bin/sh"; + return Environment.ProcessPath ?? "/bin/sh"; } } } From fc0fd30aad754d6c470d2bc8565375eedcecd9ec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 17:16:30 +0000 Subject: [PATCH 07/33] Refactor: ProcessStartOptions.ResolvePath as main impl, Process calls it Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Unix.cs | 51 +-------------- .../System/Diagnostics/ProcessStartOptions.cs | 64 ++++++++++--------- 2 files changed, 37 insertions(+), 78 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index b2fe96562dfcd9..3d1ab17a9abb1d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -446,7 +446,7 @@ private bool StartCore(ProcessStartInfo startInfo) } else { - filename = ResolvePath(startInfo.FileName); + filename = ProcessStartOptions.ResolvePath(startInfo.FileName); argv = ParseArgv(startInfo); if (Directory.Exists(filename)) { @@ -682,59 +682,12 @@ private static string[] CreateEnvp(ProcessStartInfo psi) } } - /// Resolves a path to the filename passed to ProcessStartInfo. - /// The filename. - /// The resolved path. It can return null in case of URLs. - internal static string? ResolvePath(string filename) - { - // Follow the same resolution that Windows uses with CreateProcess: - // 1. First try the exact path provided - // 2. Then try the file relative to the executable directory - // 3. Then try the file relative to the current directory - // 4. then try the file in each of the directories specified in PATH - // Windows does additional Windows-specific steps between 3 and 4, - // and we ignore those here. - - // If the filename is a complete path, use it, regardless of whether it exists. - if (Path.IsPathRooted(filename)) - { - // In this case, it doesn't matter whether the file exists or not; - // it's what the caller asked for, so it's what they'll get - return filename; - } - - // Then check the executable's directory - string? path = Environment.ProcessPath; - if (path != null) - { - try - { - path = Path.Combine(Path.GetDirectoryName(path)!, filename); - if (File.Exists(path)) - { - return path; - } - } - catch (ArgumentException) { } // ignore any errors in data that may come from the exe path - } - - // Then check the current directory - path = Path.Combine(Directory.GetCurrentDirectory(), filename); - if (File.Exists(path)) - { - return path; - } - - // Then check each directory listed in the PATH environment variables - return FindProgramInPath(filename); - } - /// /// Gets the path to the program /// /// /// - internal static string? FindProgramInPath(string program) + private static string? FindProgramInPath(string program) { string path; string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index b70b042bc96ca5..1cdac7cbbbb042 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,11 +31,7 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set - { - ArgumentNullException.ThrowIfNull(value); - _arguments = value; - } + set => _arguments = value; } /// @@ -95,11 +91,7 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set - { - ArgumentNullException.ThrowIfNull(value); - _inheritedHandles = value; - } + set => _inheritedHandles = value; } /// @@ -142,21 +134,6 @@ public ProcessStartOptions(string fileName) /// Thrown when cannot be resolved to an existing file. internal static string ResolvePath(string filename) { -#if WINDOWS - return ResolvePathWindows(filename); -#else - string? resolved = Process.ResolvePath(filename); - if (resolved == null) - { - throw new FileNotFoundException("Could not resolve the file.", filename); - } - return resolved; -#endif - } - -#if WINDOWS - private static string ResolvePathWindows(string filename) - { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) { @@ -165,6 +142,7 @@ private static string ResolvePathWindows(string filename) return filename; } +#if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. // Therefore, if the file name extension is .com, this parameter must include the .com extension. @@ -175,6 +153,7 @@ private static string ResolvePathWindows(string filename) { filename += ".exe"; } +#endif // Then check the executable's directory string? path = System.Environment.ProcessPath; @@ -183,7 +162,11 @@ private static string ResolvePathWindows(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -193,11 +176,16 @@ private static string ResolvePathWindows(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } +#if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) @@ -229,9 +217,10 @@ private static string ResolvePathWindows(string filename) return path; } } +#endif // Then check each directory listed in the PATH environment variables - return FindProgramInPathWindows(filename); + return FindProgramInPath(filename); } /// @@ -240,17 +229,22 @@ private static string ResolvePathWindows(string filename) /// The program name. /// The full path to the program. /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPathWindows(string program) + private static string FindProgramInPath(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { - var pathParser = new StringParser(pathEnvVar, ';', skipEmpty: true); +#if WINDOWS + char pathSeparator = ';'; +#else + char pathSeparator = ':'; +#endif + var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (File.Exists(path)) + if (IsExecutableFile(path)) { return path; } @@ -259,6 +253,18 @@ private static string FindProgramInPathWindows(string program) throw new FileNotFoundException("Could not resolve the file.", program); } + + private static bool IsExecutableFile(string path) + { +#if WINDOWS + return File.Exists(path); +#else + return Process.IsExecutable(path); +#endif + } + +#if !WINDOWS + // Unix executable checking is in Process.IsExecutable #endif #if WINDOWS From f687eb8f1620ea24a22d2b902ffc47c56516ff78 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 11 Feb 2026 19:19:59 +0100 Subject: [PATCH 08/33] address my own feedback --- .../src/System/Diagnostics/Process.Linux.cs | 2 +- .../src/System/Diagnostics/Process.SunOS.cs | 2 +- .../src/System/Diagnostics/Process.Unix.cs | 27 +------- .../System/Diagnostics/ProcessStartOptions.cs | 64 ++++++------------- 4 files changed, 23 insertions(+), 72 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs index 5cf8c5c76a993d..a47c8885afa9e4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs @@ -108,7 +108,7 @@ private static DateTime BootTime ReadOnlySpan allowedProgramsToRun = ["xdg-open", "gnome-open", "kfmclient"]; foreach (var program in allowedProgramsToRun) { - string? pathToProgram = FindProgramInPath(program); + string? pathToProgram = ProcessStartOptions.FindProgramInPath(program); if (!string.IsNullOrEmpty(pathToProgram)) { return pathToProgram; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs index f40076a3882aca..3a3bff81abd0a4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs @@ -38,7 +38,7 @@ internal DateTime StartTimeCore /// Gets execution path private static string? GetPathToOpenFile() { - return FindProgramInPath("xdg-open"); + return ProcessStartOptions.FindProgramInPath("xdg-open"); } /// diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 3d1ab17a9abb1d..fc46f14f918e7b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -663,7 +663,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) // find filename on PATH else { - resolvedFilename = FindProgramInPath(filename); + resolvedFilename = ProcessStartOptions.FindProgramInPath(filename); } } @@ -682,31 +682,6 @@ private static string[] CreateEnvp(ProcessStartInfo psi) } } - /// - /// Gets the path to the program - /// - /// - /// - private static string? FindProgramInPath(string program) - { - string path; - string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); - if (pathEnvVar != null) - { - var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true); - while (pathParser.MoveNext()) - { - string subPath = pathParser.ExtractCurrent(); - path = Path.Combine(subPath, program); - if (IsExecutable(path)) - { - return path; - } - } - } - return null; - } - internal static bool IsExecutable(string fullPath) { Interop.Sys.FileStatus fileinfo; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 1cdac7cbbbb042..3ba446f13dcf7a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,7 +31,11 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set => _arguments = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _arguments = value; + } } /// @@ -91,7 +95,11 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set => _inheritedHandles = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _inheritedHandles = value; + } } /// @@ -125,14 +133,10 @@ public ProcessStartOptions(string fileName) { ArgumentException.ThrowIfNullOrEmpty(fileName); - _fileName = ResolvePath(fileName); + _fileName = ResolvePath(fileName) ?? throw new FileNotFoundException("Could not resolve the file.", fileName); ; } - /// Resolves a path to the filename. - /// The filename. - /// The resolved path. - /// Thrown when cannot be resolved to an existing file. - internal static string ResolvePath(string filename) + internal static string? ResolvePath(string filename) { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) @@ -162,11 +166,7 @@ internal static string ResolvePath(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -176,11 +176,7 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -219,54 +215,34 @@ internal static string ResolvePath(string filename) } #endif - // Then check each directory listed in the PATH environment variables return FindProgramInPath(filename); } - /// - /// Gets the path to the program by searching in PATH environment variable. - /// - /// The program name. - /// The full path to the program. - /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPath(string program) + internal static string? FindProgramInPath(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { -#if WINDOWS - char pathSeparator = ';'; -#else - char pathSeparator = ':'; -#endif + char pathSeparator = OperatingSystem.IsWindows() ? ';' : ':'; var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (IsExecutableFile(path)) +#if WINDOWS + if (File.Exists(path)) +#else + if (Process.IsExecutable(path)) +#endif { return path; } } } - throw new FileNotFoundException("Could not resolve the file.", program); - } - - private static bool IsExecutableFile(string path) - { -#if WINDOWS - return File.Exists(path); -#else - return Process.IsExecutable(path); -#endif + return null; } -#if !WINDOWS - // Unix executable checking is in Process.IsExecutable -#endif - #if WINDOWS private static string? s_cachedSystemDirectory; From 5459ec9dc9124fc37bf52c4588a367864e0a67ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 18:48:29 +0000 Subject: [PATCH 09/33] Simplify Windows tests: remove ConditionalFact, fix nested try-finally Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 16 +++---- .../tests/ProcessStartOptionsTests.Windows.cs | 42 ++++--------------- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 79ba8ced885c37..7114346ef3b34f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -63,6 +63,7 @@ public void TestResolvePath_PathSeparatorIsColon() string fileName = "testscript"; string fullPath = Path.Combine(tempDir, fileName); + string oldPath = Environment.GetEnvironmentVariable("PATH"); try { File.WriteAllText(fullPath, "#!/bin/sh\necho test"); @@ -70,20 +71,13 @@ public void TestResolvePath_PathSeparatorIsColon() File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); // Add temp directory to PATH using colon separator - string oldPath = Environment.GetEnvironmentVariable("PATH"); - try - { - Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Environment.SetEnvironmentVariable("PATH", oldPath); - } + Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Environment.SetEnvironmentVariable("PATH", oldPath); if (File.Exists(fullPath)) { File.Delete(fullPath); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 287cb573fedf16..1a00288aea5caa 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -9,7 +9,7 @@ namespace System.Diagnostics.Tests { public class ProcessStartOptionsTests_Windows { - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided @@ -18,7 +18,7 @@ public void TestResolvePath_AddsExeExtension() Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." @@ -26,7 +26,7 @@ public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() Assert.Throws(() => new ProcessStartOptions("notepad.")); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_PreservesComExtension() { // The .com extension should be preserved @@ -72,37 +72,11 @@ public void TestResolvePath_FindsInSystemDirectory() [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void TestResolvePath_FindsInWindowsDirectory() { - // Some utilities are in Windows directory - // We'll test with a file that's commonly in Windows directory - // Note: This might not exist on all systems, so we make it conditional - try - { - ProcessStartOptions options = new ProcessStartOptions("notepad"); - Assert.True(File.Exists(options.FileName)); - } - catch (FileNotFoundException) - { - // Skip if notepad is not found - it's not critical for this test - } - } - - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInPath() - { - // powershell.exe should be findable in PATH on most Windows systems - try - { - ProcessStartOptions options = new ProcessStartOptions("powershell"); - Assert.True(File.Exists(options.FileName)); - Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); - } - catch (FileNotFoundException) - { - // Skip if PowerShell is not found - it might not be in PATH - } + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); @@ -135,7 +109,7 @@ public void TestResolvePath_UsesCurrentDirectory() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_PathSeparatorIsSemicolon() { // Create a temp directory and file @@ -174,7 +148,7 @@ public void TestResolvePath_PathSeparatorIsSemicolon() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); From c3517ee471e104bd4b8593e2369913a2e142c098 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 07:51:04 +0000 Subject: [PATCH 10/33] Refactor tests: partial classes, remove Test prefix, use new() syntax, Theory for utils Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 106 ++++++------------ .../tests/ProcessStartOptionsTests.Windows.cs | 92 +++++++-------- .../tests/ProcessStartOptionsTests.cs | 104 ++++++----------- 3 files changed, 106 insertions(+), 196 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 7114346ef3b34f..ee8db2da0474f9 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -7,27 +7,35 @@ namespace System.Diagnostics.Tests { [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] - public class ProcessStartOptionsTests_Unix + public partial class ProcessStartOptionsTests { [Fact] - public void TestResolvePath_FindsInPath() + public void Constructor_ResolvesShOnUnix() + { + ProcessStartOptions options = new("sh"); + Assert.True(File.Exists(options.FileName)); + Assert.Equal("/bin/sh", options.FileName); + } + + [Fact] + public void ResolvePath_FindsInPath() { // sh should be findable in PATH on all Unix systems - ProcessStartOptions options = new ProcessStartOptions("sh"); + ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); Assert.EndsWith("sh", options.FileName); } [Fact] - public void TestResolvePath_DoesNotAddExeExtension() + public void ResolvePath_DoesNotAddExeExtension() { // On Unix, no .exe extension should be added - ProcessStartOptions options = new ProcessStartOptions("sh"); + ProcessStartOptions options = new("sh"); Assert.False(options.FileName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)); } [Fact] - public void TestResolvePath_UsesCurrentDirectory() + public void ResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); string fileName = "testscript.sh"; @@ -41,8 +49,8 @@ public void TestResolvePath_UsesCurrentDirectory() File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); + ProcessStartOptions options = new(fileName); + Assert.Equal(Path.GetFullPath(fullPath), options.FileName); } finally { @@ -55,7 +63,7 @@ public void TestResolvePath_UsesCurrentDirectory() } [Fact] - public void TestResolvePath_PathSeparatorIsColon() + public void ResolvePath_PathSeparatorIsColon() { // Create a temp directory and file string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); @@ -72,8 +80,8 @@ public void TestResolvePath_PathSeparatorIsColon() // Add temp directory to PATH using colon separator Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); + ProcessStartOptions options = new(fileName); + Assert.Equal(Path.GetFullPath(fullPath), options.FileName); } finally { @@ -90,48 +98,12 @@ public void TestResolvePath_PathSeparatorIsColon() } [Fact] - public void TestResolvePath_ChecksExecutablePermissions() - { - // Create a file without execute permissions - string tempDir = Path.GetTempPath(); - string fileName = "nonexecutable.sh"; - string fullPath = Path.Combine(tempDir, fileName); - - try - { - File.WriteAllText(fullPath, "#!/bin/sh\necho test"); - // Explicitly make it non-executable - File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - // Should throw because file is not executable - Assert.Throws(() => new ProcessStartOptions(fileName)); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } - } - finally - { - if (File.Exists(fullPath)) - { - File.Delete(fullPath); - } - } - } - - [Fact] - public void TestResolvePath_AbsolutePathIsNotModified() + public void ResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); try { - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally @@ -143,43 +115,35 @@ public void TestResolvePath_AbsolutePathIsNotModified() } } - [Fact] - public void TestResolvePath_FindsCommonUtilities() + [Theory] + [InlineData("ls")] + [InlineData("cat")] + [InlineData("echo")] + [InlineData("sh")] + public void ResolvePath_FindsCommonUtilities(string utilName) { - // Test common Unix utilities - string[] commonUtils = { "ls", "cat", "echo", "sh" }; - - foreach (string util in commonUtils) - { - ProcessStartOptions options = new ProcessStartOptions(util); - Assert.True(File.Exists(options.FileName), $"{util} should be found and exist"); - Assert.Contains(util, options.FileName); - } + ProcessStartOptions options = new(utilName); + Assert.True(File.Exists(options.FileName), $"{utilName} should be found and exist"); + Assert.Contains(utilName, options.FileName); } [Fact] - public void TestResolvePath_RejectsDirectories() + public void ResolvePath_RejectsDirectories() { // Create a directory with executable permissions string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); Directory.CreateDirectory(tempDir); + string oldDir = Directory.GetCurrentDirectory(); try { // Try to use the directory name as a command - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(Path.GetTempPath()); - Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(Path.GetTempPath()); + Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); } finally { + Directory.SetCurrentDirectory(oldDir); if (Directory.Exists(tempDir)) { Directory.Delete(tempDir); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 1a00288aea5caa..fd75560d0677c0 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -7,52 +7,52 @@ namespace System.Diagnostics.Tests { - public class ProcessStartOptionsTests_Windows + public partial class ProcessStartOptionsTests { - [Fact] - public void TestResolvePath_AddsExeExtension() + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void Constructor_ResolvesCmdOnWindows() + { + ProcessStartOptions options = new("cmd"); + Assert.EndsWith("cmd.exe", options.FileName); + Assert.True(File.Exists(options.FileName)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void ResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided - ProcessStartOptions options = new ProcessStartOptions("notepad"); + ProcessStartOptions options = new("notepad"); Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); Assert.True(File.Exists(options.FileName)); } [Fact] - public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() + public void ResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." // This should fail since "notepad." won't exist - Assert.Throws(() => new ProcessStartOptions("notepad.")); + Assert.Throws(() => new("notepad.")); } [Fact] - public void TestResolvePath_PreservesComExtension() + public void ResolvePath_PreservesComExtension() { // The .com extension should be preserved string fileName = "test.com"; string tempDir = Path.GetTempPath(); string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "test"); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new(fileName); + Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -61,47 +61,39 @@ public void TestResolvePath_PreservesComExtension() } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInSystemDirectory() + public void ResolvePath_FindsInSystemDirectory() { // cmd.exe should be found in system directory - ProcessStartOptions options = new ProcessStartOptions("cmd"); + ProcessStartOptions options = new("cmd"); Assert.True(File.Exists(options.FileName)); Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInWindowsDirectory() + public void ResolvePath_FindsInWindowsDirectory() { - ProcessStartOptions options = new ProcessStartOptions("notepad"); + ProcessStartOptions options = new("notepad"); Assert.True(File.Exists(options.FileName)); } [Fact] - public void TestResolvePath_UsesCurrentDirectory() + public void ResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); string fileName = "testapp.exe"; string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "test"); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -110,7 +102,7 @@ public void TestResolvePath_UsesCurrentDirectory() } [Fact] - public void TestResolvePath_PathSeparatorIsSemicolon() + public void ResolvePath_PathSeparatorIsSemicolon() { // Create a temp directory and file string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); @@ -118,25 +110,17 @@ public void TestResolvePath_PathSeparatorIsSemicolon() string fileName = "testexe.exe"; string fullPath = Path.Combine(tempDir, fileName); + string oldPath = Environment.GetEnvironmentVariable("PATH"); try { File.WriteAllText(fullPath, "test"); - - // Add temp directory to PATH using semicolon separator - string oldPath = Environment.GetEnvironmentVariable("PATH"); - try - { - Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); - ProcessStartOptions options = new ProcessStartOptions("testexe"); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Environment.SetEnvironmentVariable("PATH", oldPath); - } + Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); + ProcessStartOptions options = new("testexe"); + Assert.Equal(fullPath, options.FileName); } finally { + Environment.SetEnvironmentVariable("PATH", oldPath); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -149,7 +133,7 @@ public void TestResolvePath_PathSeparatorIsSemicolon() } [Fact] - public void TestResolvePath_AbsolutePathIsNotModified() + public void ResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); try @@ -159,7 +143,7 @@ public void TestResolvePath_AbsolutePathIsNotModified() File.Move(tempFile, noExtFile); tempFile = noExtFile; - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index 4bd075a8227948..75b361e3337877 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -10,51 +10,34 @@ namespace System.Diagnostics.Tests { - public class ProcessStartOptionsTests : ProcessTestBase + public partial class ProcessStartOptionsTests { [Fact] - public void TestConstructor_NullFileName_Throws() + public void Constructor_NullFileName_Throws() { Assert.Throws(() => new ProcessStartOptions(null)); } [Fact] - public void TestConstructor_EmptyFileName_Throws() + public void Constructor_EmptyFileName_Throws() { Assert.Throws(() => new ProcessStartOptions(string.Empty)); } [Fact] - public void TestConstructor_NonExistentFile_Throws() + public void Constructor_NonExistentFile_Throws() { string nonExistentFile = "ThisFileDoesNotExist_" + Guid.NewGuid().ToString(); Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestConstructor_ResolvesCmdOnWindows() - { - ProcessStartOptions options = new ProcessStartOptions("cmd"); - Assert.EndsWith("cmd.exe", options.FileName); - Assert.True(File.Exists(options.FileName)); - } - - [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] [Fact] - public void TestConstructor_ResolvesShOnUnix() - { - ProcessStartOptions options = new ProcessStartOptions("sh"); - Assert.Contains("sh", options.FileName); - Assert.True(File.Exists(options.FileName)); - } - - [Fact] - public void TestConstructor_WithAbsolutePath() + public void Constructor_WithAbsolutePath() { string tempFile = Path.GetTempFileName(); try { - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally @@ -64,18 +47,18 @@ public void TestConstructor_WithAbsolutePath() } [Fact] - public void TestArguments_DefaultIsEmpty() + public void Arguments_DefaultIsEmpty() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IList args = options.Arguments; Assert.NotNull(args); Assert.Empty(args); } [Fact] - public void TestArguments_CanAddAndModify() + public void Arguments_CanAddAndModify() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.Arguments.Add("arg1"); options.Arguments.Add("arg2"); Assert.Equal(2, options.Arguments.Count); @@ -88,24 +71,9 @@ public void TestArguments_CanAddAndModify() } [Fact] - public void TestEnvironment_LazyInitialization() + public void Environment_CanAddAndModify() { - if (PlatformDetection.IsiOS || PlatformDetection.IstvOS || PlatformDetection.IsMacCatalyst) - { - // Whole list of environment variables can no longer be accessed on non-OSX apple platforms - return; - } - - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); - IDictionary env = options.Environment; - Assert.NotNull(env); - Assert.NotEmpty(env); - } - - [Fact] - public void TestEnvironment_CanAddAndModify() - { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IDictionary env = options.Environment; int originalCount = env.Count; @@ -121,9 +89,9 @@ public void TestEnvironment_CanAddAndModify() } [Fact] - public void TestEnvironment_CaseInsensitivityOnWindows() + public void Environment_CaseSensitivityIsPlatformSpecific() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IDictionary env = options.Environment; env["TestKey"] = "TestValue"; @@ -140,80 +108,74 @@ public void TestEnvironment_CaseInsensitivityOnWindows() } [Fact] - public void TestInheritedHandles_DefaultIsEmpty() + public void InheritedHandles_DefaultIsEmpty() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IList handles = options.InheritedHandles; Assert.NotNull(handles); Assert.Empty(handles); } [Fact] - public void TestInheritedHandles_CanSet() + public void InheritedHandles_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); List newHandles = new List(); options.InheritedHandles = newHandles; Assert.Same(newHandles, options.InheritedHandles); } [Fact] - public void TestWorkingDirectory_DefaultIsNull() + public void WorkingDirectory_DefaultIsNull() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.Null(options.WorkingDirectory); } [Fact] - public void TestWorkingDirectory_CanSet() + public void WorkingDirectory_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); string tempDir = Path.GetTempPath(); options.WorkingDirectory = tempDir; Assert.Equal(tempDir, options.WorkingDirectory); } [Fact] - public void TestKillOnParentExit_DefaultIsFalse() + public void KillOnParentExit_DefaultIsFalse() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.False(options.KillOnParentExit); } [Fact] - public void TestKillOnParentExit_CanSet() + public void KillOnParentExit_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.KillOnParentExit = true; Assert.True(options.KillOnParentExit); } [Fact] - public void TestCreateNewProcessGroup_DefaultIsFalse() + public void CreateNewProcessGroup_DefaultIsFalse() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.False(options.CreateNewProcessGroup); } [Fact] - public void TestCreateNewProcessGroup_CanSet() + public void CreateNewProcessGroup_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.CreateNewProcessGroup = true; Assert.True(options.CreateNewProcessGroup); } private string GetCurrentProcessName() { - // Get a valid executable path for testing - if (OperatingSystem.IsWindows()) - { - return Path.Combine(Environment.SystemDirectory, "cmd.exe"); - } - else - { - return Environment.ProcessPath ?? "/bin/sh"; - } + return Environment.ProcessPath ?? (OperatingSystem.IsWindows() + ? Path.Combine(Environment.SystemDirectory, "cmd.exe") + : "/bin/sh"); } } } From bbea91e0a3a5eaaafe1e7b24460f40d9e3a5dd3b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 08:03:00 +0000 Subject: [PATCH 11/33] Fix test conditions: remove redundant IsWindows check, use GetFullPath for sh Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 2 +- .../tests/ProcessStartOptionsTests.Windows.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index ee8db2da0474f9..820411c954e747 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,7 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal("/bin/sh", options.FileName); + Assert.Equal(Path.GetFullPath("/bin/sh"), options.FileName); } [Fact] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index fd75560d0677c0..eb9fa34225029f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -9,7 +9,7 @@ namespace System.Diagnostics.Tests { public partial class ProcessStartOptionsTests { - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void Constructor_ResolvesCmdOnWindows() { ProcessStartOptions options = new("cmd"); From d9d381fef63440991a40ecfab8197cc9cd4e6b77 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 09:53:18 +0000 Subject: [PATCH 12/33] Fix test failures: use ResolveTarget helper for symlinks, fix lambda syntax Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 10 ++++++++-- .../tests/ProcessStartOptionsTests.Windows.cs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 820411c954e747..325d580d9a6673 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,7 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(Path.GetFullPath("/bin/sh"), options.FileName); + Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); } [Fact] @@ -23,7 +23,13 @@ public void ResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.EndsWith("sh", options.FileName); + Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + } + + private static string ResolveTarget(string path) + { + FileSystemInfo? target = File.ResolveLinkTarget(path, returnFinalTarget: true); + return target?.FullName ?? path; } [Fact] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index eb9fa34225029f..06371aae1bcd88 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -31,7 +31,7 @@ public void ResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." // This should fail since "notepad." won't exist - Assert.Throws(() => new("notepad.")); + Assert.Throws(() => new ProcessStartOptions("notepad.")); } [Fact] From b4d6e6495d7c730be47f45b1a724d775894b0f45 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 10:53:03 +0000 Subject: [PATCH 13/33] Fix Unix test failures: use EndsWith instead of exact path comparison Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 325d580d9a6673..cf357680eb9da6 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,8 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + // Verify the resolved path ends with "sh" (could be /bin/sh, /usr/bin/sh, etc.) + Assert.EndsWith("sh", options.FileName); } [Fact] @@ -23,7 +24,8 @@ public void ResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + // Verify the resolved path ends with "sh" (could be /bin/sh, /usr/bin/sh, etc.) + Assert.EndsWith("sh", options.FileName); } private static string ResolveTarget(string path) From 98d420113a6a647a22812a15db5e47770c17c621 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:34:26 +0000 Subject: [PATCH 14/33] Address code review: add file check, fix .exe logic, update comments, remove dead code Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/Resources/Strings.resx | 3 +++ .../src/System/Diagnostics/ProcessStartOptions.cs | 14 +++++++++++--- .../tests/ProcessStartOptionsTests.Unix.cs | 6 ------ .../tests/ProcessStartOptionsTests.Windows.cs | 7 ++++--- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 3cc0d1e1d09e61..67d05840e9796b 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -336,4 +336,7 @@ Invalid performance counter data with type '{0}'. + + Could not resolve the file. + \ No newline at end of file diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 3ba446f13dcf7a..1d65eb12dc2076 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -133,7 +133,12 @@ public ProcessStartOptions(string fileName) { ArgumentException.ThrowIfNullOrEmpty(fileName); - _fileName = ResolvePath(fileName) ?? throw new FileNotFoundException("Could not resolve the file.", fileName); ; + string? resolved = ResolvePath(fileName); + if (resolved == null || !File.Exists(resolved)) + { + throw new FileNotFoundException(SR.FileNotFoundResolvePath, fileName); + } + _fileName = resolved; } internal static string? ResolvePath(string filename) @@ -153,7 +158,9 @@ public ProcessStartOptions(string fileName) // If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended." // HasExtension returns false for trailing dot, so we need to check that separately - if (filename[filename.Length - 1] != '.' && !Path.HasExtension(filename)) + if (filename[filename.Length - 1] != '.' + && string.IsNullOrEmpty(Path.GetDirectoryName(filename)) + && !Path.HasExtension(filename)) { filename += ".exe"; } @@ -262,7 +269,8 @@ public ProcessStartOptions(string fileName) private static string? GetWindowsDirectory() { - // Don't cache Windows directory as it's user-specific and can change during app lifetime + // We don't cache the Windows directory; GetWindowsDirectoryW returns a stable system-wide path, + // and this method is not expected to be called frequently enough to warrant additional caching. Span buffer = stackalloc char[260]; // MAX_PATH uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); if (length > 0 && length < buffer.Length) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index cf357680eb9da6..22a2c535077f6a 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -28,12 +28,6 @@ public void ResolvePath_FindsInPath() Assert.EndsWith("sh", options.FileName); } - private static string ResolveTarget(string path) - { - FileSystemInfo? target = File.ResolveLinkTarget(path, returnFinalTarget: true); - return target?.FullName ?? path; - } - [Fact] public void ResolvePath_DoesNotAddExeExtension() { diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 06371aae1bcd88..1600c71e5ebfb4 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -17,7 +17,7 @@ public void Constructor_ResolvesCmdOnWindows() Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), nameof(PlatformDetection.IsNotWindowsServerCore))] public void ResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided @@ -66,10 +66,11 @@ public void ResolvePath_FindsInSystemDirectory() // cmd.exe should be found in system directory ProcessStartOptions options = new("cmd"); Assert.True(File.Exists(options.FileName)); - Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); + string systemDirectory = Environment.SystemDirectory; + Assert.StartsWith(systemDirectory, options.FileName, StringComparison.OrdinalIgnoreCase); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), nameof(PlatformDetection.IsNotWindowsServerCore))] public void ResolvePath_FindsInWindowsDirectory() { ProcessStartOptions options = new("notepad"); From 5b024d4a24cdb6d65c2fb48f15258b0f6ca79657 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:54:27 +0000 Subject: [PATCH 15/33] Cache Windows directory value for performance Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System/Diagnostics/ProcessStartOptions.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 1d65eb12dc2076..90d5acbf378f63 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -252,6 +252,7 @@ public ProcessStartOptions(string fileName) #if WINDOWS private static string? s_cachedSystemDirectory; + private static string? s_cachedWindowsDirectory; private static string? GetSystemDirectory() { @@ -269,15 +270,16 @@ public ProcessStartOptions(string fileName) private static string? GetWindowsDirectory() { - // We don't cache the Windows directory; GetWindowsDirectoryW returns a stable system-wide path, - // and this method is not expected to be called frequently enough to warrant additional caching. - Span buffer = stackalloc char[260]; // MAX_PATH - uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); - if (length > 0 && length < buffer.Length) + if (s_cachedWindowsDirectory == null) { - return new string(buffer.Slice(0, (int)length)); + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + s_cachedWindowsDirectory = new string(buffer.Slice(0, (int)length)); + } } - return null; + return s_cachedWindowsDirectory; } #endif } From add4af64bb0ef4b754ca90ac8a8b1dfab3210f61 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 19:54:45 +0000 Subject: [PATCH 16/33] Remove DictionaryWrapper and fix Unix path test for macOS symlinks Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 6 +++--- .../tests/ProcessStartOptionsTests.Unix.cs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 90d5acbf378f63..fb12746af8399a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -17,7 +17,7 @@ public sealed class ProcessStartOptions { private readonly string _fileName; private IList? _arguments; - private DictionaryWrapper? _environment; + private Dictionary? _environment; private IList? _inheritedHandles; /// @@ -52,9 +52,9 @@ public IList Arguments { IDictionary envVars = System.Environment.GetEnvironmentVariables(); - _environment = new DictionaryWrapper(new Dictionary( + _environment = new Dictionary( envVars.Count, - OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal)); + OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations. IDictionaryEnumerator e = envVars.GetEnumerator(); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 22a2c535077f6a..9aae9937f37f7d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,7 +52,8 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - Assert.Equal(Path.GetFullPath(fullPath), options.FileName); + // Use Path.GetFullPath on both sides to handle symlinks (e.g., /tmp -> /private/tmp on macOS) + Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); } finally { From b3939ad14ad133fa41aaf445ac839d814d12230b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 06:23:42 +0000 Subject: [PATCH 17/33] Use Environment.SystemDirectory and fix Unix path test for macOS Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 11 +---------- .../tests/ProcessStartOptionsTests.Unix.cs | 5 +++-- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index fb12746af8399a..30eb189f480f1d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -256,16 +256,7 @@ public ProcessStartOptions(string fileName) private static string? GetSystemDirectory() { - if (s_cachedSystemDirectory == null) - { - Span buffer = stackalloc char[260]; // MAX_PATH - uint length = Interop.Kernel32.GetSystemDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); - if (length > 0 && length < buffer.Length) - { - s_cachedSystemDirectory = new string(buffer.Slice(0, (int)length)); - } - } - return s_cachedSystemDirectory; + return s_cachedSystemDirectory ??= System.Environment.SystemDirectory; } private static string? GetWindowsDirectory() diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 9aae9937f37f7d..0c6060135af691 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,8 +52,9 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - // Use Path.GetFullPath on both sides to handle symlinks (e.g., /tmp -> /private/tmp on macOS) - Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); + // options.FileName is already the resolved full path + // Use Path.GetFullPath to canonicalize fullPath (handles /tmp -> /private/tmp on macOS) + Assert.Equal(Path.GetFullPath(fullPath), options.FileName); } finally { From 63e7fc0fbb448ee1c526f10cad9b0d87d6aa4687 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 06:37:48 +0000 Subject: [PATCH 18/33] Inline GetSystemDirectory, fix Unix path test, use EndsWith for util names Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 2 +- .../tests/ProcessStartOptionsTests.Unix.cs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 30eb189f480f1d..4d9045c30c1064 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -192,7 +192,7 @@ public ProcessStartOptions(string fileName) // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) - path = GetSystemDirectory(); + path = s_cachedSystemDirectory ??= System.Environment.SystemDirectory; if (path != null) { path = Path.Combine(path, filename); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 0c6060135af691..8ab1fd04bafe76 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,9 +52,9 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - // options.FileName is already the resolved full path - // Use Path.GetFullPath to canonicalize fullPath (handles /tmp -> /private/tmp on macOS) - Assert.Equal(Path.GetFullPath(fullPath), options.FileName); + // Path.GetFullPath resolves symlinks in the path (e.g., /tmp -> /private/tmp on macOS) + // options.FileName is already fully resolved by ProcessStartOptions + Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); } finally { @@ -128,7 +128,7 @@ public void ResolvePath_FindsCommonUtilities(string utilName) { ProcessStartOptions options = new(utilName); Assert.True(File.Exists(options.FileName), $"{utilName} should be found and exist"); - Assert.Contains(utilName, options.FileName); + Assert.EndsWith(utilName, options.FileName); } [Fact] From 9ceaad179af3f483fa4b29b83edc55c6a2412118 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 13 Feb 2026 08:26:24 +0100 Subject: [PATCH 19/33] Update src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj --- .../src/System.Diagnostics.Process.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 78eee4897b0b89..53e635042d4e21 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -140,8 +140,6 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetProcessPriorityBoost.cs" /> - Date: Fri, 13 Feb 2026 12:33:17 +0100 Subject: [PATCH 20/33] fix the ResolvePath_UsesCurrentDirectory test --- .../tests/ProcessStartOptionsTests.Unix.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 8ab1fd04bafe76..71b6ed911a9a7f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,9 +52,10 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - // Path.GetFullPath resolves symlinks in the path (e.g., /tmp -> /private/tmp on macOS) - // options.FileName is already fully resolved by ProcessStartOptions - Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); + + Assert.True(File.Exists(options.FileName)); + // on macOS, we need to handle /tmp/testscript.sh -> /private/tmp/testscript.sh + Assert.EndsWith(fullPath, options.FileName); } finally { From 27ad99b4968aaf1dbcd8ec14c2a3db19dc4921e5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 11:57:35 +0000 Subject: [PATCH 21/33] Initial plan From 90ba78839504dfa94226b1be6fec849a7c86d065 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:24:36 +0000 Subject: [PATCH 22/33] Add ProcessExitStatus type, Windows interop files, and ProcessUtils Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Interop.GenerateConsoleCtrlEvent.cs | 17 +++++ .../Kernel32/Interop.GetHandleInformation.cs | 15 ++++ .../Windows/Kernel32/Interop.JobObjects.cs | 70 +++++++++++++++++++ .../Interop.ProcThreadAttributeList.cs | 49 +++++++++++++ .../Kernel32/Interop.ResumeThread_IntPtr.cs | 14 ++++ .../System/Diagnostics/ProcessExitStatus.cs | 63 +++++++++++++++++ .../Diagnostics/ProcessUtils.Windows.cs | 57 +++++++++++++++ 7 files changed, 285 insertions(+) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenerateConsoleCtrlEvent.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcThreadAttributeList.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread_IntPtr.cs create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessExitStatus.cs create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenerateConsoleCtrlEvent.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenerateConsoleCtrlEvent.cs new file mode 100644 index 00000000000000..5add2e5a992f46 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenerateConsoleCtrlEvent.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + internal const int CTRL_C_EVENT = 0; + internal const int CTRL_BREAK_EVENT = 1; + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GenerateConsoleCtrlEvent(int dwCtrlEvent, int dwProcessGroupId); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs new file mode 100644 index 00000000000000..20d1470dd6a1cb --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetHandleInformation(IntPtr hObject, out int lpdwFlags); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs new file mode 100644 index 00000000000000..ecdc85253f9387 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs @@ -0,0 +1,70 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + internal static partial IntPtr CreateJobObjectW(IntPtr lpJobAttributes, IntPtr lpName); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool AssignProcessToJobObject(IntPtr hJob, IntPtr hProcess); + + internal const uint JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE = 0x00002000; + + internal enum JOBOBJECTINFOCLASS + { + JobObjectBasicLimitInformation = 2, + JobObjectExtendedLimitInformation = 9 + } + + [StructLayout(LayoutKind.Sequential)] + internal struct IO_COUNTERS + { + internal ulong ReadOperationCount; + internal ulong WriteOperationCount; + internal ulong OtherOperationCount; + internal ulong ReadTransferCount; + internal ulong WriteTransferCount; + internal ulong OtherTransferCount; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct JOBOBJECT_BASIC_LIMIT_INFORMATION + { + internal long PerProcessUserTimeLimit; + internal long PerJobUserTimeLimit; + internal uint LimitFlags; + internal UIntPtr MinimumWorkingSetSize; + internal UIntPtr MaximumWorkingSetSize; + internal uint ActiveProcessLimit; + internal UIntPtr Affinity; + internal uint PriorityClass; + internal uint SchedulingClass; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct JOBOBJECT_EXTENDED_LIMIT_INFORMATION + { + internal JOBOBJECT_BASIC_LIMIT_INFORMATION BasicLimitInformation; + internal IO_COUNTERS IoInfo; + internal UIntPtr ProcessMemoryLimit; + internal UIntPtr JobMemoryLimit; + internal UIntPtr PeakProcessMemoryUsed; + internal UIntPtr PeakJobMemoryUsed; + } + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool SetInformationJobObject(IntPtr hJob, JOBOBJECTINFOCLASS JobObjectInfoClass, ref JOBOBJECT_EXTENDED_LIMIT_INFORMATION lpJobObjectInfo, uint cbJobObjectInfoLength); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool TerminateJobObject(IntPtr hJob, uint uExitCode); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcThreadAttributeList.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcThreadAttributeList.cs new file mode 100644 index 00000000000000..2e3e3dbcc2f841 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcThreadAttributeList.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + internal const int PROC_THREAD_ATTRIBUTE_HANDLE_LIST = 0x00020002; + internal const int PROC_THREAD_ATTRIBUTE_JOB_LIST = 0x0002000D; + internal const int EXTENDED_STARTUPINFO_PRESENT = 0x00080000; + + [StructLayout(LayoutKind.Sequential)] + internal struct STARTUPINFOEX + { + internal STARTUPINFO StartupInfo; + internal LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList; + } + + internal struct LPPROC_THREAD_ATTRIBUTE_LIST + { + internal IntPtr AttributeList; + } + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static unsafe partial bool InitializeProcThreadAttributeList( + LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList, + int dwAttributeCount, + int dwFlags, + ref IntPtr lpSize); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static unsafe partial bool UpdateProcThreadAttribute( + LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList, + int dwFlags, + IntPtr attribute, + void* lpValue, + IntPtr cbSize, + void* lpPreviousValue, + IntPtr lpReturnSize); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + internal static unsafe partial void DeleteProcThreadAttributeList(LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread_IntPtr.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread_IntPtr.cs new file mode 100644 index 00000000000000..0495f60a564f86 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ResumeThread_IntPtr.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + internal static partial int ResumeThread(IntPtr hThread); + } +} diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessExitStatus.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessExitStatus.cs new file mode 100644 index 00000000000000..b95e8eb015aaf2 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessExitStatus.cs @@ -0,0 +1,63 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices; + +namespace System.Diagnostics +{ + /// + /// Represents the exit status of a process, including exit code, signal information, and cancellation state. + /// + public readonly struct ProcessExitStatus : IEquatable + { + /// + /// Initializes a new instance of the struct. + /// + /// The exit code of the process. + /// Whether the process was terminated due to timeout or cancellation. + /// The signal that caused the process to exit, if any. + internal ProcessExitStatus(int exitCode, bool canceled, PosixSignal? signal = null) + { + ExitCode = exitCode; + Canceled = canceled; + Signal = signal; + } + + /// + /// Gets the exit code of the process. + /// + public int ExitCode { get; } + + /// + /// Gets the signal that caused the process to exit, if any. + /// + public PosixSignal? Signal { get; } + + /// + /// Gets a value indicating whether the process has been terminated due to timeout or cancellation. + /// + public bool Canceled { get; } + + /// + public bool Equals(ProcessExitStatus other) => + ExitCode == other.ExitCode && Signal == other.Signal && Canceled == other.Canceled; + + /// + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is ProcessExitStatus other && Equals(other); + + /// + /// Determines whether two instances are equal. + /// + public static bool operator ==(ProcessExitStatus left, ProcessExitStatus right) => left.Equals(right); + + /// + /// Determines whether two instances are not equal. + /// + public static bool operator !=(ProcessExitStatus left, ProcessExitStatus right) => !left.Equals(right); + + /// + public override int GetHashCode() => HashCode.Combine(ExitCode, Signal, Canceled); + } +} diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs new file mode 100644 index 00000000000000..e472e4109922c9 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs @@ -0,0 +1,57 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Text; + +namespace System.Diagnostics +{ + internal static class ProcessUtils + { + internal static void BuildArgs(ProcessStartOptions options, ref ValueStringBuilder applicationName, ref ValueStringBuilder commandLine) + { + string absolutePath = options.FileName; + + applicationName.Append(absolutePath); + applicationName.NullTerminate(); + + // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw + // "Because argv[0] is the module name, C programmers generally repeat the module name as the first token in the command line." + // The truth is that some programs REQUIRE it (example: findstr). That is why we repeat it. + PasteArguments.AppendArgument(ref commandLine, options.FileName); + + foreach (string argument in options.Arguments) + { + PasteArguments.AppendArgument(ref commandLine, argument); + } + commandLine.NullTerminate(); + } + + internal static string GetEnvironmentVariablesBlock(IDictionary sd) + { + // https://learn.microsoft.com/windows/win32/procthread/changing-environment-variables + // "All strings in the environment block must be sorted alphabetically by name. The sort is + // case-insensitive, Unicode order, without regard to locale. Because the equal sign is a + // separator, it must not be used in the name of an environment variable." + + var keys = new string[sd.Count]; + sd.Keys.CopyTo(keys, 0); + Array.Sort(keys, StringComparer.OrdinalIgnoreCase); + + // Join the null-terminated "key=val\0" strings + var result = new StringBuilder(8 * keys.Length); + foreach (string key in keys) + { + string? value = sd[key]; + + // Ignore null values for consistency with Environment.SetEnvironmentVariable + if (value != null) + { + result.Append(key).Append('=').Append(value).Append('\0'); + } + } + + return result.ToString(); + } + } +} From 23a457a7444288a64d8402dcaf589eac76dd56b1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:34:19 +0000 Subject: [PATCH 23/33] Add SafeProcessHandle new APIs, ProcessExitStatus, interop definitions, and Windows implementation Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Advapi32/Interop.ProcessOptions.cs | 1 + .../Kernel32/Interop.GetHandleInformation.cs | 4 + .../Windows/Kernel32/Interop.HandleOptions.cs | 1 + .../Interop.ProcThreadAttributeList.cs | 15 + .../ref/System.Diagnostics.Process.cs | 25 + .../SafeHandles/SafeProcessHandle.Unix.cs | 42 +- .../SafeHandles/SafeProcessHandle.Windows.cs | 554 +++++++++++++++++- .../Win32/SafeHandles/SafeProcessHandle.cs | 286 ++++++++- .../src/Resources/Strings.resx | 24 + .../src/System.Diagnostics.Process.csproj | 12 + .../System/Diagnostics/ProcessStartOptions.cs | 10 + 11 files changed, 943 insertions(+), 31 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 893a0aaed9073c..95daea39e5fdf2 100644 --- a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs +++ b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs @@ -44,6 +44,7 @@ internal static partial class StartupInfoOptions internal const int CREATE_UNICODE_ENVIRONMENT = 0x00000400; internal const int CREATE_NO_WINDOW = 0x08000000; internal const int CREATE_NEW_PROCESS_GROUP = 0x00000200; + internal const int CREATE_SUSPENDED = 0x00000004; } } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs index 20d1470dd6a1cb..14be2003f4c26c 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs @@ -11,5 +11,9 @@ internal static partial class Kernel32 [LibraryImport(Libraries.Kernel32, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] internal static partial bool GetHandleInformation(IntPtr hObject, out int lpdwFlags); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool SetHandleInformation(IntPtr hObject, int dwMask, int dwFlags); } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.HandleOptions.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.HandleOptions.cs index 4a2b32f7c3c301..182ba49df5ca4c 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.HandleOptions.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.HandleOptions.cs @@ -10,6 +10,7 @@ internal static partial class HandleOptions internal const int DUPLICATE_SAME_ACCESS = 2; internal const int STILL_ACTIVE = 0x00000103; internal const int TOKEN_ADJUST_PRIVILEGES = 0x20; + internal const int HANDLE_FLAG_INHERIT = 0x00000001; } } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcThreadAttributeList.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcThreadAttributeList.cs index 2e3e3dbcc2f841..7887a5de1a64f1 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcThreadAttributeList.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcThreadAttributeList.cs @@ -24,6 +24,21 @@ internal struct LPPROC_THREAD_ATTRIBUTE_LIST internal IntPtr AttributeList; } + [LibraryImport(Libraries.Kernel32, EntryPoint = "CreateProcessW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static unsafe partial bool CreateProcess( + char* lpApplicationName, + char* lpCommandLine, + ref SECURITY_ATTRIBUTES procSecAttrs, + ref SECURITY_ATTRIBUTES threadSecAttrs, + [MarshalAs(UnmanagedType.Bool)] bool bInheritHandles, + int dwCreationFlags, + char* lpEnvironment, + string? lpCurrentDirectory, + ref STARTUPINFOEX lpStartupInfo, + ref PROCESS_INFORMATION lpProcessInformation + ); + [LibraryImport(Libraries.Kernel32, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] internal static unsafe partial bool InitializeProcThreadAttributeList( 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 c6b6565016ea69..d87a8c62b7a0b7 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -10,7 +10,21 @@ public sealed partial class SafeProcessHandle : Microsoft.Win32.SafeHandles.Safe { public SafeProcessHandle() : base (default(bool)) { } public SafeProcessHandle(System.IntPtr existingHandle, bool ownsHandle) : base (default(bool)) { } + public int ProcessId { get { throw null; } } + public bool Kill() { throw null; } + public bool KillProcessGroup() { throw null; } + public static Microsoft.Win32.SafeHandles.SafeProcessHandle Open(int processId) { throw null; } protected override bool ReleaseHandle() { throw null; } + public void Resume() { } + public void Signal(System.Runtime.InteropServices.PosixSignal signal) { } + public void SignalProcessGroup(System.Runtime.InteropServices.PosixSignal signal) { } + public static Microsoft.Win32.SafeHandles.SafeProcessHandle Start(System.Diagnostics.ProcessStartOptions options, Microsoft.Win32.SafeHandles.SafeFileHandle? input, Microsoft.Win32.SafeHandles.SafeFileHandle? output, Microsoft.Win32.SafeHandles.SafeFileHandle? error) { throw null; } + public static Microsoft.Win32.SafeHandles.SafeProcessHandle StartSuspended(System.Diagnostics.ProcessStartOptions options, Microsoft.Win32.SafeHandles.SafeFileHandle? input, Microsoft.Win32.SafeHandles.SafeFileHandle? output, Microsoft.Win32.SafeHandles.SafeFileHandle? error) { throw null; } + public bool TryWaitForExit(System.TimeSpan timeout, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Diagnostics.ProcessExitStatus? exitStatus) { throw null; } + public System.Diagnostics.ProcessExitStatus WaitForExit() { throw null; } + public System.Threading.Tasks.Task WaitForExitAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + public System.Threading.Tasks.Task WaitForExitOrKillOnCancellationAsync(System.Threading.CancellationToken cancellationToken) { throw null; } + public System.Diagnostics.ProcessExitStatus WaitForExitOrKillOnTimeout(System.TimeSpan timeout) { throw null; } } } namespace System.Diagnostics @@ -272,6 +286,17 @@ public ProcessStartOptions(string fileName) { } public bool KillOnParentExit { get { throw null; } set { } } public string? WorkingDirectory { get { throw null; } set { } } } + public readonly partial struct ProcessExitStatus : System.IEquatable + { + public bool Canceled { get { throw null; } } + public int ExitCode { get { throw null; } } + public System.Runtime.InteropServices.PosixSignal? Signal { get { throw null; } } + public bool Equals(System.Diagnostics.ProcessExitStatus other) { throw null; } + public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; } + public override int GetHashCode() { throw null; } + public static bool operator ==(System.Diagnostics.ProcessExitStatus left, System.Diagnostics.ProcessExitStatus right) { throw null; } + public static bool operator !=(System.Diagnostics.ProcessExitStatus left, System.Diagnostics.ProcessExitStatus right) { throw null; } + } [System.ComponentModel.DesignerAttribute("System.Diagnostics.Design.ProcessThreadDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class ProcessThread : System.ComponentModel.Component { diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index f3cac1f1af898b..5f28b91bebac9c 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -1,17 +1,14 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -/*============================================================ -** -** Class: SafeProcessHandle -** -** A wrapper for a process handle -** -** -===========================================================*/ - using System; +using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; namespace Microsoft.Win32.SafeHandles { @@ -35,16 +32,39 @@ internal SafeProcessHandle(int processId, SafeWaitHandle handle) : handle.DangerousAddRef(ref _releaseRef); } - internal int ProcessId { get; } + /// + /// Gets the process ID. + /// + public int ProcessId { get; private set; } protected override bool ReleaseHandle() { if (_releaseRef) { - Debug.Assert(_handle != null); + Debug.Assert(_handle is not null); _handle.DangerousRelease(); } return true; } + + private static SafeProcessHandle OpenCore(int processId) => throw new NotImplementedException(); + + private static SafeProcessHandle StartCore(ProcessStartOptions options, SafeFileHandle inputHandle, SafeFileHandle outputHandle, SafeFileHandle errorHandle, bool createSuspended) => throw new NotImplementedException(); + + private ProcessExitStatus WaitForExitCore() => throw new NotImplementedException(); + + private bool TryWaitForExitCore(int milliseconds, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) => throw new NotImplementedException(); + + private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) => throw new NotImplementedException(); + + private Task WaitForExitAsyncCore(CancellationToken cancellationToken) => throw new NotImplementedException(); + + private Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) => throw new NotImplementedException(); + + internal bool KillCore(bool throwOnError, bool entireProcessGroup = false) => throw new NotImplementedException(); + + private void ResumeCore() => throw new NotImplementedException(); + + private void SendSignalCore(PosixSignal signal, bool entireProcessGroup) => throw new NotImplementedException(); } } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index 1fc7a409713278..2266d37d94f6a7 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -1,26 +1,560 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -/*============================================================ -** -** Class: SafeProcessHandle -** -** A wrapper for a process handle -** -** -===========================================================*/ - using System; +using System.ComponentModel; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Security; +using System.Text; +using System.Threading; +using System.Threading.Tasks; namespace Microsoft.Win32.SafeHandles { public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvalid { + // Static job object used for KillOnParentExit functionality + // All child processes with KillOnParentExit=true are assigned to this job + // Note: The job handle is intentionally never closed - it should live for the + // lifetime of the process. When this process exits, the job object is destroyed + // by the OS, which terminates all child processes in the job. + private static readonly Lazy s_killOnParentExitJob = new(CreateKillOnParentExitJob); + + // Thread handle for suspended processes (only used on Windows) + private IntPtr _threadHandle; + + // Job handle for CreateNewProcessGroup functionality (only used on Windows) + // This is specific to each process and is used to terminate the entire process group + private IntPtr _processGroupJobHandle; + + /// + /// Gets the process ID. + /// + public int ProcessId { get; private set; } + + private SafeProcessHandle(IntPtr processHandle, IntPtr threadHandle, IntPtr processGroupJobHandle, int processId) + : base(ownsHandle: true) + { + SetHandle(processHandle); + _threadHandle = threadHandle; + _processGroupJobHandle = processGroupJobHandle; + ProcessId = processId; + } + + private static IntPtr CreateKillOnParentExitJob() + { + IntPtr jobHandle = Interop.Kernel32.CreateJobObjectW(IntPtr.Zero, IntPtr.Zero); + if (jobHandle == IntPtr.Zero) + { + throw new Win32Exception(Marshal.GetLastPInvokeError(), SR.Format(SR.CouldNotCreateJobObject)); + } + + Interop.Kernel32.JOBOBJECT_EXTENDED_LIMIT_INFORMATION limitInfo = default; + limitInfo.BasicLimitInformation.LimitFlags = Interop.Kernel32.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + + if (!Interop.Kernel32.SetInformationJobObject( + jobHandle, + Interop.Kernel32.JOBOBJECTINFOCLASS.JobObjectExtendedLimitInformation, + ref limitInfo, + (uint)Marshal.SizeOf())) + { + int error = Marshal.GetLastPInvokeError(); + Interop.Kernel32.CloseHandle(jobHandle); + throw new Win32Exception(error, SR.Format(SR.CouldNotSetJobObjectInfo)); + } + + return jobHandle; + } + protected override bool ReleaseHandle() { + if (_threadHandle != IntPtr.Zero) + { + Interop.Kernel32.CloseHandle(_threadHandle); + } + + if (_processGroupJobHandle != IntPtr.Zero) + { + Interop.Kernel32.CloseHandle(_processGroupJobHandle); + } + return Interop.Kernel32.CloseHandle(handle); } + + internal int GetExitCode() + { + if (!Interop.Kernel32.GetExitCodeProcess(this, out int exitCode)) + { + throw new InvalidOperationException(SR.CouldNotGetExitCode); + } + else if (exitCode == Interop.Kernel32.HandleOptions.STILL_ACTIVE) + { + throw new InvalidOperationException(SR.ProcessNotExited); + } + + return exitCode; + } + + private bool TryGetExitCodeCore(out int exitCode, out PosixSignal? signal) + { + signal = default; + + return Interop.Kernel32.GetExitCodeProcess(this, out exitCode) + && exitCode != Interop.Kernel32.HandleOptions.STILL_ACTIVE; + } + + private static unsafe SafeProcessHandle StartCore(ProcessStartOptions options, SafeFileHandle inputHandle, SafeFileHandle outputHandle, SafeFileHandle errorHandle, bool createSuspended) + { + ValueStringBuilder applicationName = new(stackalloc char[256]); + ValueStringBuilder commandLine = new(stackalloc char[256]); + ProcessUtils.BuildArgs(options, ref applicationName, ref commandLine); + + Interop.Kernel32.STARTUPINFOEX startupInfoEx = default; + Interop.Kernel32.PROCESS_INFORMATION processInfo = default; + Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = default; + SafeProcessHandle? procSH = null; + IntPtr currentProcHandle = Interop.Kernel32.GetCurrentProcess(); + IntPtr attributeListBuffer = IntPtr.Zero; + Interop.Kernel32.LPPROC_THREAD_ATTRIBUTE_LIST attributeList = default; + + using SafeFileHandle duplicatedInput = Duplicate(inputHandle, currentProcHandle); + using SafeFileHandle duplicatedOutput = inputHandle.DangerousGetHandle() == outputHandle.DangerousGetHandle() + ? duplicatedInput + : Duplicate(outputHandle, currentProcHandle); + using SafeFileHandle duplicatedError = outputHandle.DangerousGetHandle() == errorHandle.DangerousGetHandle() + ? duplicatedOutput + : (inputHandle.DangerousGetHandle() == errorHandle.DangerousGetHandle() + ? duplicatedInput + : Duplicate(errorHandle, currentProcHandle)); + + int maxHandleCount = 3 + (options.HasInheritedHandlesBeenAccessed ? options.InheritedHandles.Count : 0); + + IntPtr heapHandlesPtr = Marshal.AllocHGlobal(maxHandleCount * sizeof(IntPtr)); + IntPtr* handlesToInherit = (IntPtr*)heapHandlesPtr; + IntPtr processGroupJobHandle = IntPtr.Zero; + + try + { + int handleCount = 0; + + IntPtr inputPtr = duplicatedInput.DangerousGetHandle(); + IntPtr outputPtr = duplicatedOutput.DangerousGetHandle(); + IntPtr errorPtr = duplicatedError.DangerousGetHandle(); + + PrepareHandleAllowList(options, handlesToInherit, ref handleCount, inputPtr, outputPtr, errorPtr); + + if (options.CreateNewProcessGroup) + { + processGroupJobHandle = Interop.Kernel32.CreateJobObjectW(IntPtr.Zero, IntPtr.Zero); + if (processGroupJobHandle == IntPtr.Zero) + { + throw new Win32Exception(Marshal.GetLastPInvokeError(), SR.Format(SR.CouldNotCreateJobObject)); + } + } + + int attributeCount = 1; // Always need handle list + if (options.KillOnParentExit || options.CreateNewProcessGroup) + attributeCount++; + + IntPtr size = IntPtr.Zero; + Interop.Kernel32.LPPROC_THREAD_ATTRIBUTE_LIST emptyList = default; + Interop.Kernel32.InitializeProcThreadAttributeList(emptyList, attributeCount, 0, ref size); + + attributeListBuffer = Marshal.AllocHGlobal(size); + attributeList.AttributeList = attributeListBuffer; + + if (!Interop.Kernel32.InitializeProcThreadAttributeList(attributeList, attributeCount, 0, ref size)) + { + throw new Win32Exception(); + } + + if (!Interop.Kernel32.UpdateProcThreadAttribute( + attributeList, + 0, + (IntPtr)Interop.Kernel32.PROC_THREAD_ATTRIBUTE_HANDLE_LIST, + handlesToInherit, + (IntPtr)(handleCount * sizeof(IntPtr)), + null, + IntPtr.Zero)) + { + throw new Win32Exception(); + } + + if (options.KillOnParentExit || options.CreateNewProcessGroup) + { + IntPtr* pJobHandle = stackalloc IntPtr[2]; + int jobsCount = 0; + + if (options.KillOnParentExit) + pJobHandle[jobsCount++] = s_killOnParentExitJob.Value; + if (options.CreateNewProcessGroup) + pJobHandle[jobsCount++] = processGroupJobHandle; + + if (!Interop.Kernel32.UpdateProcThreadAttribute( + attributeList, + 0, + Interop.Kernel32.PROC_THREAD_ATTRIBUTE_JOB_LIST, + pJobHandle, + jobsCount * sizeof(IntPtr), + null, + IntPtr.Zero)) + { + throw new Win32Exception(Marshal.GetLastPInvokeError()); + } + } + + startupInfoEx.lpAttributeList = attributeList; + startupInfoEx.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFOEX); + startupInfoEx.StartupInfo.hStdInput = inputPtr; + startupInfoEx.StartupInfo.hStdOutput = outputPtr; + startupInfoEx.StartupInfo.hStdError = errorPtr; + startupInfoEx.StartupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; + + int creationFlags = Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT; + if (createSuspended) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_SUSPENDED; + if (options.CreateNewProcessGroup) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NEW_PROCESS_GROUP; + + string? environmentBlock = null; + if (options.HasEnvironmentBeenAccessed) + { + creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT; + environmentBlock = ProcessUtils.GetEnvironmentVariablesBlock(options.Environment); + } + + string? workingDirectory = options.WorkingDirectory; + int errorCode = 0; + + fixed (char* environmentBlockPtr = environmentBlock) + fixed (char* applicationNamePtr = &applicationName.GetPinnableReference()) + fixed (char* commandLinePtr = &commandLine.GetPinnableReference()) + { + bool retVal = Interop.Kernel32.CreateProcess( + applicationNamePtr, + commandLinePtr, + ref unused_SecAttrs, + ref unused_SecAttrs, + true, + creationFlags, + environmentBlockPtr, + workingDirectory, + ref startupInfoEx, + ref processInfo + ); + if (!retVal) + errorCode = Marshal.GetLastPInvokeError(); + } + + if (processInfo.hProcess != IntPtr.Zero && processInfo.hProcess != new IntPtr(-1)) + { + if (createSuspended && processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) + { + procSH = new(processInfo.hProcess, processInfo.hThread, processGroupJobHandle, processInfo.dwProcessId); + } + else + { + procSH = new(processInfo.hProcess, IntPtr.Zero, processGroupJobHandle, processInfo.dwProcessId); + if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) + Interop.Kernel32.CloseHandle(processInfo.hThread); + } + } + else if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) + { + Interop.Kernel32.CloseHandle(processInfo.hThread); + } + + if (procSH is null) + { + throw new Win32Exception(errorCode); + } + } + catch + { + procSH?.Dispose(); + + if (processGroupJobHandle != IntPtr.Zero) + { + Interop.Kernel32.CloseHandle(processGroupJobHandle); + } + + throw; + } + finally + { + Marshal.FreeHGlobal(heapHandlesPtr); + + if (attributeListBuffer != IntPtr.Zero) + { + Interop.Kernel32.DeleteProcThreadAttributeList(attributeList); + Marshal.FreeHGlobal(attributeListBuffer); + } + Interop.Kernel32.CloseHandle(currentProcHandle); + } + + return procSH; + + static SafeFileHandle Duplicate(SafeFileHandle sourceHandle, nint currentProcHandle) + { + if (!Interop.Kernel32.DuplicateHandle( + currentProcHandle, + sourceHandle, + currentProcHandle, + out SafeFileHandle duplicated, + 0, + true, + Interop.Kernel32.HandleOptions.DUPLICATE_SAME_ACCESS)) + { + throw new Win32Exception(); + } + + return duplicated; + } + } + + private static unsafe void PrepareHandleAllowList(ProcessStartOptions options, IntPtr* handlesToInherit, ref int handleCount, IntPtr inputPtr, IntPtr outputPtr, IntPtr errorPtr) + { + handlesToInherit[handleCount++] = inputPtr; + if (outputPtr != inputPtr) + handlesToInherit[handleCount++] = outputPtr; + if (errorPtr != inputPtr && errorPtr != outputPtr) + handlesToInherit[handleCount++] = errorPtr; + + if (options.HasInheritedHandlesBeenAccessed) + { + foreach (SafeHandle handle in options.InheritedHandles) + { + IntPtr handlePtr = handle.DangerousGetHandle(); + + bool isDuplicate = false; + for (int i = 0; i < handleCount; i++) + { + if (handlesToInherit[i] == handlePtr) + { + isDuplicate = true; + break; + } + } + + if (!isDuplicate) + { + if (!Interop.Kernel32.GetHandleInformation(handlePtr, out int flags)) + { + throw new Win32Exception(Marshal.GetLastPInvokeError()); + } + + if ((flags & Interop.Kernel32.HandleOptions.HANDLE_FLAG_INHERIT) == 0) + { + if (!Interop.Kernel32.SetHandleInformation( + handlePtr, + Interop.Kernel32.HandleOptions.HANDLE_FLAG_INHERIT, + Interop.Kernel32.HandleOptions.HANDLE_FLAG_INHERIT)) + { + throw new Win32Exception(Marshal.GetLastPInvokeError()); + } + } + + handlesToInherit[handleCount++] = handlePtr; + } + } + } + } + + private ProcessExitStatus WaitForExitCore() + { + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + processWaitHandle.WaitOne(Timeout.Infinite); + + return new(GetExitCode(), false); + } + + private bool TryWaitForExitCore(int milliseconds, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) + { + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + if (!processWaitHandle.WaitOne(milliseconds)) + { + exitStatus = null; + return false; + } + + exitStatus = new(GetExitCode(), false); + return true; + } + + private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) + { + bool wasKilledOnTimeout = false; + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + if (!processWaitHandle.WaitOne(milliseconds)) + { + wasKilledOnTimeout = KillCore(throwOnError: false); + } + + return new(GetExitCode(), wasKilledOnTimeout); + } + + private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) + { + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + processWaitHandle, + (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.Register( + static state => + { + var taskSource = (TaskCompletionSource)state!; + taskSource.TrySetCanceled(); + }, + tcs); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + } + + return new(GetExitCode(), false); + } + + private async Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) + { + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + StrongBox wasKilledBox = new(false); + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + processWaitHandle, + (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.Register( + static state => + { + var (handle, taskSource, wasCancelled) = ((SafeProcessHandle, TaskCompletionSource, StrongBox))state!; + wasCancelled.Value = handle.KillCore(throwOnError: false); + taskSource.TrySetResult(true); + }, + (this, tcs, wasKilledBox)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + } + + return new(GetExitCode(), wasKilledBox.Value); + } + + internal bool KillCore(bool throwOnError, bool entireProcessGroup = false) + { + if (entireProcessGroup && _processGroupJobHandle == IntPtr.Zero) + { + throw new InvalidOperationException(SR.KillProcessGroupWithoutNewProcessGroup); + } + + if (entireProcessGroup + ? Interop.Kernel32.TerminateJobObject(_processGroupJobHandle, unchecked((uint)-1)) + : Interop.Kernel32.TerminateProcess(this, exitCode: -1)) + { + return true; + } + + int error = Marshal.GetLastPInvokeError(); + return error switch + { + Interop.Errors.ERROR_SUCCESS => true, + Interop.Errors.ERROR_ACCESS_DENIED => false, + _ when !throwOnError => false, + _ => throw new Win32Exception(error), + }; + } + + private void ResumeCore() + { + if (_threadHandle == IntPtr.Zero) + { + throw new InvalidOperationException(SR.CannotResumeNonSuspendedProcess); + } + + int result = Interop.Kernel32.ResumeThread(_threadHandle); + if (result == -1) + { + int error = Marshal.GetLastPInvokeError(); + throw new Win32Exception(error); + } + } + + private void SendSignalCore(PosixSignal signal, bool entireProcessGroup) + { + if (signal == PosixSignal.SIGKILL) + { + KillCore(throwOnError: true, entireProcessGroup); + return; + } + + int ctrlEvent = signal switch + { + PosixSignal.SIGINT => Interop.Kernel32.CTRL_C_EVENT, + PosixSignal.SIGQUIT => Interop.Kernel32.CTRL_BREAK_EVENT, + _ => throw new ArgumentException(SR.Format(SR.SignalNotSupportedOnWindows, signal), nameof(signal)) + }; + + if (!Interop.Kernel32.GenerateConsoleCtrlEvent(ctrlEvent, ProcessId)) + { + int error = Marshal.GetLastPInvokeError(); + throw new Win32Exception(error); + } + } + + private static SafeProcessHandle OpenCore(int processId) + { + const int desiredAccess = Interop.Advapi32.ProcessOptions.PROCESS_QUERY_LIMITED_INFORMATION + | Interop.Advapi32.ProcessOptions.SYNCHRONIZE + | Interop.Advapi32.ProcessOptions.PROCESS_TERMINATE; + + SafeProcessHandle safeHandle = Interop.Kernel32.OpenProcess(desiredAccess, inherit: false, processId); + + if (safeHandle.IsInvalid) + { + int error = Marshal.GetLastPInvokeError(); + safeHandle.Dispose(); + throw new Win32Exception(error); + } + + // Transfer ownership: take the handle from the returned SafeProcessHandle and + // create a new one with the ProcessId set properly. + IntPtr rawHandle = safeHandle.DangerousGetHandle(); + safeHandle.SetHandleAsInvalid(); // Prevent the original from closing it + return new SafeProcessHandle(rawHandle, IntPtr.Zero, IntPtr.Zero, processId); + } } } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index c7d52e23e0b0ce..718c5403f429e9 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -1,26 +1,26 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -/*============================================================ -** -** Class: SafeProcessHandle -** -** A wrapper for a process handle -** -** -===========================================================*/ - using System; +using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; namespace Microsoft.Win32.SafeHandles { + /// + /// A wrapper for a process handle. + /// public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvalid { internal static readonly SafeProcessHandle InvalidHandle = new SafeProcessHandle(); /// - /// Creates a . + /// Creates a . /// public SafeProcessHandle() : this(IntPtr.Zero) @@ -42,5 +42,271 @@ public SafeProcessHandle(IntPtr existingHandle, bool ownsHandle) { SetHandle(existingHandle); } + + /// + /// Opens an existing child process by its process ID. + /// + /// The process ID of the process to open. + /// A that represents the opened process. + /// Thrown when is negative or zero. + /// Thrown when the process could not be opened. + /// + /// On Windows, this method uses OpenProcess with PROCESS_QUERY_LIMITED_INFORMATION, SYNCHRONIZE, and PROCESS_TERMINATE permissions. + /// On Linux with pidfd support, this method uses the pidfd_open syscall. + /// On other Unix systems, this method uses kill(pid, 0) to verify the process exists and the caller has permission to signal it. + /// + public static SafeProcessHandle Open(int processId) + { + ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(processId, 0); + + return OpenCore(processId); + } + + /// + /// Starts a new process. + /// + /// The process start options. + /// The handle to use for standard input, or to provide no input. + /// The handle to use for standard output, or to discard output. + /// The handle to use for standard error, or to discard error. + /// A handle to the started process. + /// Thrown when is null. + public static SafeProcessHandle Start(ProcessStartOptions options, SafeFileHandle? input, SafeFileHandle? output, SafeFileHandle? error) + { + return StartInternal(options, input, output, error, createSuspended: false); + } + + /// + /// Starts a new process in a suspended state. + /// + /// Process start options. + /// Standard input handle. + /// Standard output handle. + /// Standard error handle. + /// A handle to the suspended process. Call to start execution. + /// Thrown when is null. + public static SafeProcessHandle StartSuspended(ProcessStartOptions options, SafeFileHandle? input, SafeFileHandle? output, SafeFileHandle? error) + { + return StartInternal(options, input, output, error, createSuspended: true); + } + + private static SafeProcessHandle StartInternal(ProcessStartOptions options, SafeFileHandle? input, SafeFileHandle? output, SafeFileHandle? error, bool createSuspended) + { + ArgumentNullException.ThrowIfNull(options); + + SafeFileHandle? nullHandle = null; + + if (input is null || output is null || error is null) + { + nullHandle = File.OpenHandle( + OperatingSystem.IsWindows() ? "NUL" : "/dev/null", + FileMode.Open, + FileAccess.ReadWrite, + FileShare.ReadWrite); + + input ??= nullHandle; + output ??= nullHandle; + error ??= nullHandle; + } + + try + { + return StartCore(options, input, output, error, createSuspended); + } + finally + { + nullHandle?.Dispose(); + } + } + + /// + /// Waits for the process to exit without a timeout. + /// + /// The exit status of the process. + /// Thrown when the handle is invalid. + public ProcessExitStatus WaitForExit() + { + Validate(); + + return WaitForExitCore(); + } + + /// + /// Waits for the process to exit within the specified timeout. + /// + /// The maximum time to wait for the process to exit. + /// When this method returns true, contains the exit status of the process. + /// true if the process exited before the timeout; otherwise, false. + /// Thrown when the handle is invalid. + public bool TryWaitForExit(TimeSpan timeout, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) + { + Validate(); + + return TryWaitForExitCore(GetTimeoutInMilliseconds(timeout), out exitStatus); + } + + /// + /// Waits for the process to exit within the specified timeout. + /// If the process does not exit before the timeout, it is killed and then waited for exit. + /// + /// The maximum time to wait for the process to exit before killing it. + /// The exit status of the process. If the process was killed due to timeout, Canceled will be true. + /// Thrown when the handle is invalid. + public ProcessExitStatus WaitForExitOrKillOnTimeout(TimeSpan timeout) + { + Validate(); + + return WaitForExitOrKillOnTimeoutCore(GetTimeoutInMilliseconds(timeout)); + } + + /// + /// Waits asynchronously for the process to exit and reports the exit status. + /// + /// A cancellation token that can be used to cancel the wait operation. + /// A task that represents the asynchronous wait operation. The task result contains the exit status of the process. + /// Thrown when the handle is invalid. + /// Thrown when the cancellation token is canceled. + /// + /// When the cancellation token is canceled, this method stops waiting and throws . + /// The process is NOT killed and continues running. If you want to kill the process on cancellation, + /// use instead. + /// + public Task WaitForExitAsync(CancellationToken cancellationToken = default) + { + Validate(); + + return WaitForExitAsyncCore(cancellationToken); + } + + /// + /// Waits asynchronously for the process to exit and reports the exit status. + /// When cancelled, kills the process and then waits for exit without timeout. + /// + /// A cancellation token that can be used to cancel the wait operation and kill the process. + /// A task that represents the asynchronous wait operation. The task result contains the exit status of the process. + /// If the process was killed due to cancellation, the Canceled property will be true. + /// Thrown when the handle is invalid. + /// + /// When the cancellation token is canceled, this method kills the process and waits for it to exit. + /// The returned exit status will have the property set to true if the process was killed. + /// If the cancellation token cannot be canceled (e.g., ), this method behaves identically + /// to and will wait indefinitely for the process to exit. + /// + public Task WaitForExitOrKillOnCancellationAsync(CancellationToken cancellationToken) + { + Validate(); + + return WaitForExitOrKillOnCancellationAsyncCore(cancellationToken); + } + + /// + /// Terminates the process. + /// + /// + /// true if the process was terminated; false if the process had already exited. + /// + /// Thrown when the handle is invalid. + /// Thrown when the kill operation fails for reasons other than the process having already exited. + public bool Kill() + { + Validate(); + + return KillCore(throwOnError: true, entireProcessGroup: false); + } + + /// + /// Terminates the entire process group. + /// + /// + /// true if the process group was terminated; false if the process had already exited. + /// + /// Thrown when the handle is invalid. + /// Thrown when the kill operation fails for reasons other than the process having already exited. + /// + /// On Unix, sends SIGKILL to all processes in the process group. + /// On Windows, requires the process to have been started with =true. + /// Terminates all processes in the job object. If the process was not started with =true, + /// throws an . + /// + public bool KillProcessGroup() + { + Validate(); + + return KillCore(throwOnError: true, entireProcessGroup: true); + } + + /// + /// Resumes a suspended process. + /// + /// Thrown when the handle is invalid. + /// Thrown when the resume operation fails. + public void Resume() + { + Validate(); + + ResumeCore(); + } + + /// + /// Sends a signal to the process. + /// + /// The signal to send. + /// Thrown when the handle is invalid. + /// Thrown when the signal value is not supported. + /// Thrown when the signal is not supported on the current platform. + /// Thrown when the signal operation fails. + /// + /// On Windows, only SIGINT (mapped to CTRL_C_EVENT), SIGQUIT (mapped to CTRL_BREAK_EVENT), and SIGKILL are supported. + /// The process must have been started with set to true for signals to work properly. + /// On Windows, signals are always sent to the entire process group, not just the single process. + /// On Unix/Linux, all signals defined in PosixSignal are supported, and the signal is sent only to the specific process. + /// + public void Signal(PosixSignal signal) + { + if (!Enum.IsDefined(signal)) + { + throw new ArgumentOutOfRangeException(nameof(signal)); + } + + Validate(); + + SendSignalCore(signal, entireProcessGroup: false); + } + + /// + /// Sends a signal to the entire process group. + /// + /// The signal to send. + /// Thrown when the handle is invalid. + /// Thrown when the signal value is not supported. + /// Thrown when the signal operation fails. + /// + /// On Windows, only SIGINT (mapped to CTRL_C_EVENT), SIGQUIT (mapped to CTRL_BREAK_EVENT), and SIGKILL are supported. + /// The process must have been started with set to true for signals to work properly. + /// On Windows, signals are always sent to the entire process group. + /// On Unix/Linux, all signals defined in PosixSignal are supported, and the signal is sent to all processes in the process group. + /// + public void SignalProcessGroup(PosixSignal signal) + { + if (!Enum.IsDefined(signal)) + { + throw new ArgumentOutOfRangeException(nameof(signal)); + } + + Validate(); + + SendSignalCore(signal, entireProcessGroup: true); + } + + private void Validate() + { + if (IsInvalid) + { + throw new InvalidOperationException(SR.InvalidProcessHandle); + } + } + + internal static int GetTimeoutInMilliseconds(TimeSpan timeout) => + timeout == Timeout.InfiniteTimeSpan ? Timeout.Infinite : (int)timeout.TotalMilliseconds; } } diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 67d05840e9796b..7b035657c71c32 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -339,4 +339,28 @@ Could not resolve the file. + + Invalid process handle. + + + Failed to create job object. + + + Failed to set job object information. + + + Failed to get exit code of the process. + + + Process has not exited yet. + + + Cannot terminate entire process group because the process was not started with CreateNewProcessGroup=true. + + + Cannot resume a process that was not started with StartSuspended. + + + Signal {0} is not supported on Windows. Only SIGINT, SIGQUIT, and SIGKILL are supported. + \ No newline at end of file diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 53e635042d4e21..81a7501305e36b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -21,6 +21,7 @@ + @@ -225,6 +226,17 @@ + + + + + + diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 4d9045c30c1064..b13cfc464950df 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -107,6 +107,16 @@ public IList InheritedHandles /// public bool KillOnParentExit { get; set; } + /// + /// Gets a value indicating whether the property has been accessed. + /// + internal bool HasEnvironmentBeenAccessed => _environment is not null; + + /// + /// Gets a value indicating whether the property has been accessed. + /// + internal bool HasInheritedHandlesBeenAccessed => _inheritedHandles is not null; + /// /// Gets or sets a value indicating whether to create the process in a new process group. /// From 30177310d10bda5e1a90ce8c188a698b1cd43e13 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:49:27 +0000 Subject: [PATCH 24/33] Add test files for SafeProcessHandle APIs Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/SafeProcessHandleTests.Windows.cs | 173 +++++++++ .../tests/SafeProcessHandleTests.cs | 329 ++++++++++++++++++ .../System.Diagnostics.Process.Tests.csproj | 2 + 3 files changed, 504 insertions(+) create mode 100644 src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs new file mode 100644 index 00000000000000..c46543bad3d8a7 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs @@ -0,0 +1,173 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using System.Runtime.InteropServices; +using System.Threading.Tasks; +using Microsoft.Win32.SafeHandles; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public partial class SafeProcessHandleTests + { + [DllImport("kernel32.dll")] + private static extern IntPtr GetStdHandle(int nStdHandle); + + private const int STD_INPUT_HANDLE = -10; + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] + public void SendSignal_SIGINT_TerminatesProcessInNewProcessGroup() + { + if (Console.IsInputRedirected) + { + return; + } + + ProcessStartOptions options = new("timeout") + { + Arguments = { "/t", "3", "/nobreak" }, + CreateNewProcessGroup = true + }; + + using SafeFileHandle stdin = new(GetStdHandle(STD_INPUT_HANDLE), ownsHandle: false); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: stdin, output: null, error: null); + + bool hasExited = processHandle.TryWaitForExit(TimeSpan.Zero, out _); + Assert.False(hasExited, "Process should still be running before signal is sent"); + + processHandle.Signal(PosixSignal.SIGINT); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(3000)); + + Assert.NotEqual(0, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] + public void Signal_SIGQUIT_TerminatesProcessInNewProcessGroup() + { + if (Console.IsInputRedirected) + { + return; + } + + ProcessStartOptions options = new("timeout") + { + Arguments = { "/t", "3", "/nobreak" }, + CreateNewProcessGroup = true + }; + + using SafeFileHandle stdin = new(GetStdHandle(STD_INPUT_HANDLE), ownsHandle: false); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: stdin, output: null, error: null); + + bool hasExited = processHandle.TryWaitForExit(TimeSpan.Zero, out _); + Assert.False(hasExited, "Process should still be running before signal is sent"); + + processHandle.Signal(PosixSignal.SIGQUIT); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(3000)); + + Assert.NotEqual(0, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] + public void Signal_UnsupportedSignal_ThrowsArgumentException() + { + if (Console.IsInputRedirected) + { + return; + } + + ProcessStartOptions options = new("timeout") + { + Arguments = { "/t", "3", "/nobreak" }, + CreateNewProcessGroup = true + }; + + using SafeFileHandle stdin = new(GetStdHandle(STD_INPUT_HANDLE), ownsHandle: false); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: stdin, output: null, error: null); + + try + { + Assert.Throws(() => processHandle.Signal(PosixSignal.SIGTERM)); + } + finally + { + processHandle.Kill(); + processHandle.WaitForExit(); + } + } + + [Fact] + public void CreateNewProcessGroup_CanBeSetToTrue() + { + ProcessStartOptions options = new("cmd.exe") + { + Arguments = { "/c", "echo test" }, + CreateNewProcessGroup = true + }; + + Assert.True(options.CreateNewProcessGroup); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(5)); + Assert.Equal(0, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] + public void Kill_EntireProcessGroup_WithoutCreateNewProcessGroup_Throws() + { + if (Console.IsInputRedirected) + { + return; + } + + ProcessStartOptions options = new("timeout") + { + Arguments = { "/t", "3", "/nobreak" }, + CreateNewProcessGroup = false + }; + + using SafeFileHandle stdin = new(GetStdHandle(STD_INPUT_HANDLE), ownsHandle: false); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: stdin, output: null, error: null); + + Assert.Throws(() => processHandle.KillProcessGroup()); + + Assert.True(processHandle.Kill()); + } + + [Fact] + public void StartSuspended_ResumeCompletes() + { + ProcessStartOptions options = new("cmd.exe") + { + Arguments = { "/c", "echo test" } + }; + + using SafeProcessHandle processHandle = SafeProcessHandle.StartSuspended(options, input: null, output: null, error: null); + + bool hasExited = processHandle.TryWaitForExit(TimeSpan.FromMilliseconds(200), out _); + Assert.False(hasExited, "Suspended process should not have exited yet"); + + processHandle.Resume(); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(5)); + Assert.Equal(0, exitStatus.ExitCode); + } + + [Fact] + public void Resume_OnNonSuspendedProcess_ThrowsInvalidOperationException() + { + ProcessStartOptions options = new("cmd.exe") + { + Arguments = { "/c", "echo test" } + }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + Assert.Throws(() => processHandle.Resume()); + + processHandle.WaitForExit(); + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs new file mode 100644 index 00000000000000..f7a804abab1038 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -0,0 +1,329 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.IO; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Win32.SafeHandles; +using Xunit; + +namespace System.Diagnostics.Tests +{ + [PlatformSpecific(TestPlatforms.Windows)] + public partial class SafeProcessHandleTests + { + [Fact] + public static void GetProcessId_ReturnsValidPid() + { + ProcessStartOptions info = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(info, input: null, output: null, error: null); + int pid = processHandle.ProcessId; + + Assert.NotEqual(0, pid); + Assert.NotEqual(-1, pid); + Assert.True(pid > 0, "Process ID should be a positive integer"); + + nint handleValue = processHandle.DangerousGetHandle(); + Assert.NotEqual(handleValue, (nint)pid); + + ProcessExitStatus exitStatus = processHandle.WaitForExit(); + Assert.Equal(0, exitStatus.ExitCode); + Assert.Null(exitStatus.Signal); + Assert.False(exitStatus.Canceled); + } + + [Fact] + public static void Kill_KillsRunningProcess() + { + ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + bool wasKilled = processHandle.Kill(); + Assert.True(wasKilled); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(5)); + Assert.False(exitStatus.Canceled); + Assert.Equal(-1, exitStatus.ExitCode); + } + + [Fact] + public static void Kill_CanBeCalledMultipleTimes() + { + ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + bool firstKill = processHandle.Kill(); + Assert.True(firstKill); + + _ = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(5)); + + bool secondKill = processHandle.Kill(); + Assert.False(secondKill); + } + + [Fact] + public static void WaitForExit_Called_After_Kill_ReturnsExitCodeImmediately() + { + ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + bool wasKilled = processHandle.Kill(); + Assert.True(wasKilled); + + Stopwatch stopwatch = Stopwatch.StartNew(); + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(3)); + + Assert.InRange(stopwatch.Elapsed, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + Assert.False(exitStatus.Canceled); + Assert.NotEqual(0, exitStatus.ExitCode); + } + + [Fact] + public static void Kill_OnAlreadyExitedProcess_ReturnsFalse() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + ProcessExitStatus exitStatus = processHandle.WaitForExit(); + Assert.Equal(0, exitStatus.ExitCode); + + bool wasKilled = processHandle.Kill(); + Assert.False(wasKilled); + } + + [Fact] + public static void WaitForExitOrKillOnTimeout_KillsOnTimeout() + { + ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + Stopwatch stopwatch = Stopwatch.StartNew(); + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(300)); + + Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(290), TimeSpan.FromMilliseconds(2000)); + Assert.True(exitStatus.Canceled); + Assert.NotEqual(0, exitStatus.ExitCode); + } + + [Fact] + public static void WaitForExit_WaitsIndefinitelyForProcessToComplete() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + Stopwatch stopwatch = Stopwatch.StartNew(); + ProcessExitStatus exitStatus = processHandle.WaitForExit(); + stopwatch.Stop(); + + Assert.Equal(0, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [Fact] + public static void TryWaitForExit_ReturnsTrueWhenProcessExitsBeforeTimeout() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + bool exited = processHandle.TryWaitForExit(TimeSpan.FromSeconds(5), out ProcessExitStatus? exitStatus); + + Assert.True(exited); + Assert.NotNull(exitStatus); + Assert.Equal(0, exitStatus.Value.ExitCode); + Assert.False(exitStatus.Value.Canceled); + Assert.Null(exitStatus.Value.Signal); + } + + [Fact] + public static void TryWaitForExit_ReturnsFalseWhenProcessDoesNotExitBeforeTimeout() + { + ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + try + { + Stopwatch stopwatch = Stopwatch.StartNew(); + bool exited = processHandle.TryWaitForExit(TimeSpan.FromMilliseconds(300), out ProcessExitStatus? exitStatus); + stopwatch.Stop(); + + Assert.False(exited); + Assert.Null(exitStatus); + Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(290), TimeSpan.FromMilliseconds(2000)); + } + finally + { + processHandle.Kill(); + processHandle.WaitForExit(); + } + } + + [Fact] + public static void WaitForExitOrKillOnTimeout_DoesNotKillWhenProcessExitsBeforeTimeout() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(5)); + + Assert.Equal(0, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled, "Process should not be marked as canceled when it exits normally before timeout"); + Assert.Null(exitStatus.Signal); + } + + [Fact] + public static void WaitForExitOrKillOnTimeout_KillsAndWaitsWhenTimeoutOccurs() + { + ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + Stopwatch stopwatch = Stopwatch.StartNew(); + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(300)); + stopwatch.Stop(); + + Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(290), TimeSpan.FromSeconds(2)); + Assert.True(exitStatus.Canceled, "Process should be marked as canceled when killed due to timeout"); + Assert.NotEqual(0, exitStatus.ExitCode); + Assert.Equal(-1, exitStatus.ExitCode); + } + + [Fact] + public static async Task WaitForExitOrKillOnCancellationAsync_KillsOnCancellation() + { + ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 5" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + Stopwatch stopwatch = Stopwatch.StartNew(); + using CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(300)); + + ProcessExitStatus exitStatus = await processHandle.WaitForExitOrKillOnCancellationAsync(cts.Token); + + Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(270), TimeSpan.FromSeconds(2)); + Assert.True(exitStatus.Canceled); + Assert.NotEqual(0, exitStatus.ExitCode); + } + + [Fact] + public static async Task WaitForExitAsync_ThrowsOnCancellation() + { + ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 5" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + try + { + Stopwatch stopwatch = Stopwatch.StartNew(); + using CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(300)); + + await Assert.ThrowsAnyAsync(async () => + await processHandle.WaitForExitAsync(cts.Token)); + + stopwatch.Stop(); + + Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(250), TimeSpan.FromMilliseconds(2000)); + + bool hasExited = processHandle.TryWaitForExit(TimeSpan.Zero, out _); + Assert.False(hasExited, "Process should still be running after cancellation"); + } + finally + { + processHandle.Kill(); + } + } + + [Fact] + public static async Task WaitForExitAsync_CompletesNormallyWhenProcessExits() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + using CancellationTokenSource cts = new(TimeSpan.FromSeconds(5)); + ProcessExitStatus exitStatus = await processHandle.WaitForExitAsync(cts.Token); + + Assert.Equal(0, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [Fact] + public static async Task WaitForExitAsync_WithoutCancellationToken_CompletesNormally() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + ProcessExitStatus exitStatus = await processHandle.WaitForExitAsync(); + + Assert.Equal(0, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [Fact] + public static async Task WaitForExitOrKillOnCancellationAsync_CompletesNormallyWhenProcessExits() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + using CancellationTokenSource cts = new(TimeSpan.FromSeconds(1)); + ProcessExitStatus exitStatus = await processHandle.WaitForExitOrKillOnCancellationAsync(cts.Token); + + Assert.Equal(0, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [Fact] + public static void KillOnParentExit_CanBeSetToTrue() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" }, KillOnParentExit = true }; + + Assert.True(options.KillOnParentExit); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(5)); + Assert.Equal(0, exitStatus.ExitCode); + } + + [Fact] + public static void KillOnParentExit_DefaultsToFalse() + { + ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo test" } }; + Assert.False(options.KillOnParentExit); + } + + [Fact] + public static void Open_InvalidProcessId_Throws() + { + Assert.Throws(() => SafeProcessHandle.Open(0)); + Assert.Throws(() => SafeProcessHandle.Open(-1)); + } + + [Fact] + public static void Open_CurrentProcess_Succeeds() + { + int currentPid = Environment.ProcessId; + using SafeProcessHandle handle = SafeProcessHandle.Open(currentPid); + + Assert.False(handle.IsInvalid); + Assert.Equal(currentPid, handle.ProcessId); + } + } +} 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 63523ef48c86fb..7d2de878df2463 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 @@ -35,6 +35,7 @@ + @@ -44,6 +45,7 @@ + Date: Fri, 13 Feb 2026 15:45:22 +0000 Subject: [PATCH 25/33] Address review feedback: consolidate interop files, reuse existing APIs, fix Win32Exception usage, improve tests Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Interop.GenerateConsoleCtrlEvent.cs | 17 ----- .../Kernel32/Interop.GetHandleInformation.cs | 19 ----- .../Kernel32/Interop.HandleInformation.cs | 8 ++ .../Windows/Kernel32/Interop.JobObjects.cs | 4 - .../Kernel32/Interop.SetConsoleCtrlHandler.cs | 4 + .../SafeHandles/SafeProcessHandle.Windows.cs | 75 ++++++++++--------- .../Win32/SafeHandles/SafeProcessHandle.cs | 6 +- .../src/Resources/Strings.resx | 12 --- .../src/System.Diagnostics.Process.csproj | 8 +- .../src/System/Diagnostics/Process.Windows.cs | 2 +- .../Diagnostics/ProcessUtils.Windows.cs | 28 ------- .../tests/Interop.cs | 4 - .../tests/ProcessTests.Windows.cs | 2 +- .../tests/SafeProcessHandleTests.Windows.cs | 14 +--- .../tests/SafeProcessHandleTests.cs | 50 +++++-------- 15 files changed, 81 insertions(+), 172 deletions(-) delete mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenerateConsoleCtrlEvent.cs delete mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenerateConsoleCtrlEvent.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenerateConsoleCtrlEvent.cs deleted file mode 100644 index 5add2e5a992f46..00000000000000 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenerateConsoleCtrlEvent.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; - -internal static partial class Interop -{ - internal static partial class Kernel32 - { - internal const int CTRL_C_EVENT = 0; - internal const int CTRL_BREAK_EVENT = 1; - - [LibraryImport(Libraries.Kernel32, SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - internal static partial bool GenerateConsoleCtrlEvent(int dwCtrlEvent, int dwProcessGroupId); - } -} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs deleted file mode 100644 index 14be2003f4c26c..00000000000000 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetHandleInformation.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Runtime.InteropServices; - -internal static partial class Interop -{ - internal static partial class Kernel32 - { - [LibraryImport(Libraries.Kernel32, SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - internal static partial bool GetHandleInformation(IntPtr hObject, out int lpdwFlags); - - [LibraryImport(Libraries.Kernel32, SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - internal static partial bool SetHandleInformation(IntPtr hObject, int dwMask, int dwFlags); - } -} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.HandleInformation.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.HandleInformation.cs index 57ae67529f36bb..4352f0aaa91f53 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.HandleInformation.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.HandleInformation.cs @@ -20,5 +20,13 @@ internal enum HandleFlags : uint [LibraryImport(Libraries.Kernel32, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] internal static partial bool SetHandleInformation(SafeHandle hObject, HandleFlags dwMask, HandleFlags dwFlags); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetHandleInformation(IntPtr hObject, out int lpdwFlags); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool SetHandleInformation(IntPtr hObject, int dwMask, int dwFlags); } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs index ecdc85253f9387..d88ebb9707cf25 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.JobObjects.cs @@ -11,10 +11,6 @@ internal static partial class Kernel32 [LibraryImport(Libraries.Kernel32, SetLastError = true)] internal static partial IntPtr CreateJobObjectW(IntPtr lpJobAttributes, IntPtr lpName); - [LibraryImport(Libraries.Kernel32, SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - internal static partial bool AssignProcessToJobObject(IntPtr hJob, IntPtr hProcess); - internal const uint JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE = 0x00002000; internal enum JOBOBJECTINFOCLASS diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetConsoleCtrlHandler.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetConsoleCtrlHandler.cs index 63d2db5a2f0744..88c78c214f4b48 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetConsoleCtrlHandler.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetConsoleCtrlHandler.cs @@ -16,5 +16,9 @@ internal static partial class Kernel32 [LibraryImport(Libraries.Kernel32, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] internal static unsafe partial bool SetConsoleCtrlHandler(delegate* unmanaged handler, [MarshalAs(UnmanagedType.Bool)] bool Add); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GenerateConsoleCtrlEvent(int dwCtrlEvent, int dwProcessGroupId); } } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index 2266d37d94f6a7..a2aeff9aa686ab 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -49,7 +49,7 @@ private static IntPtr CreateKillOnParentExitJob() IntPtr jobHandle = Interop.Kernel32.CreateJobObjectW(IntPtr.Zero, IntPtr.Zero); if (jobHandle == IntPtr.Zero) { - throw new Win32Exception(Marshal.GetLastPInvokeError(), SR.Format(SR.CouldNotCreateJobObject)); + throw new Win32Exception(); } Interop.Kernel32.JOBOBJECT_EXTENDED_LIMIT_INFORMATION limitInfo = default; @@ -61,9 +61,8 @@ private static IntPtr CreateKillOnParentExitJob() ref limitInfo, (uint)Marshal.SizeOf())) { - int error = Marshal.GetLastPInvokeError(); Interop.Kernel32.CloseHandle(jobHandle); - throw new Win32Exception(error, SR.Format(SR.CouldNotSetJobObjectInfo)); + throw new Win32Exception(); } return jobHandle; @@ -88,11 +87,11 @@ internal int GetExitCode() { if (!Interop.Kernel32.GetExitCodeProcess(this, out int exitCode)) { - throw new InvalidOperationException(SR.CouldNotGetExitCode); + throw new Win32Exception(); } else if (exitCode == Interop.Kernel32.HandleOptions.STILL_ACTIVE) { - throw new InvalidOperationException(SR.ProcessNotExited); + throw new InvalidOperationException(); } return exitCode; @@ -108,10 +107,6 @@ private bool TryGetExitCodeCore(out int exitCode, out PosixSignal? signal) private static unsafe SafeProcessHandle StartCore(ProcessStartOptions options, SafeFileHandle inputHandle, SafeFileHandle outputHandle, SafeFileHandle errorHandle, bool createSuspended) { - ValueStringBuilder applicationName = new(stackalloc char[256]); - ValueStringBuilder commandLine = new(stackalloc char[256]); - ProcessUtils.BuildArgs(options, ref applicationName, ref commandLine); - Interop.Kernel32.STARTUPINFOEX startupInfoEx = default; Interop.Kernel32.PROCESS_INFORMATION processInfo = default; Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = default; @@ -151,7 +146,7 @@ private static unsafe SafeProcessHandle StartCore(ProcessStartOptions options, S processGroupJobHandle = Interop.Kernel32.CreateJobObjectW(IntPtr.Zero, IntPtr.Zero); if (processGroupJobHandle == IntPtr.Zero) { - throw new Win32Exception(Marshal.GetLastPInvokeError(), SR.Format(SR.CouldNotCreateJobObject)); + throw new Win32Exception(); } } @@ -202,7 +197,7 @@ private static unsafe SafeProcessHandle StartCore(ProcessStartOptions options, S null, IntPtr.Zero)) { - throw new Win32Exception(Marshal.GetLastPInvokeError()); + throw new Win32Exception(); } } @@ -221,30 +216,42 @@ private static unsafe SafeProcessHandle StartCore(ProcessStartOptions options, S if (options.HasEnvironmentBeenAccessed) { creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT; - environmentBlock = ProcessUtils.GetEnvironmentVariablesBlock(options.Environment); + environmentBlock = Process.GetEnvironmentVariablesBlock(options.Environment); } string? workingDirectory = options.WorkingDirectory; int errorCode = 0; - fixed (char* environmentBlockPtr = environmentBlock) - fixed (char* applicationNamePtr = &applicationName.GetPinnableReference()) - fixed (char* commandLinePtr = &commandLine.GetPinnableReference()) + ValueStringBuilder applicationName = new(stackalloc char[256]); + ValueStringBuilder commandLine = new(stackalloc char[256]); + try { - bool retVal = Interop.Kernel32.CreateProcess( - applicationNamePtr, - commandLinePtr, - ref unused_SecAttrs, - ref unused_SecAttrs, - true, - creationFlags, - environmentBlockPtr, - workingDirectory, - ref startupInfoEx, - ref processInfo - ); - if (!retVal) - errorCode = Marshal.GetLastPInvokeError(); + ProcessUtils.BuildArgs(options, ref applicationName, ref commandLine); + + fixed (char* environmentBlockPtr = environmentBlock) + fixed (char* applicationNamePtr = &applicationName.GetPinnableReference()) + fixed (char* commandLinePtr = &commandLine.GetPinnableReference()) + { + bool retVal = Interop.Kernel32.CreateProcess( + applicationNamePtr, + commandLinePtr, + ref unused_SecAttrs, + ref unused_SecAttrs, + true, + creationFlags, + environmentBlockPtr, + workingDirectory, + ref startupInfoEx, + ref processInfo + ); + if (!retVal) + errorCode = Marshal.GetLastPInvokeError(); + } + } + finally + { + applicationName.Dispose(); + commandLine.Dispose(); } if (processInfo.hProcess != IntPtr.Zero && processInfo.hProcess != new IntPtr(-1)) @@ -341,7 +348,7 @@ private static unsafe void PrepareHandleAllowList(ProcessStartOptions options, I { if (!Interop.Kernel32.GetHandleInformation(handlePtr, out int flags)) { - throw new Win32Exception(Marshal.GetLastPInvokeError()); + throw new Win32Exception(); } if ((flags & Interop.Kernel32.HandleOptions.HANDLE_FLAG_INHERIT) == 0) @@ -351,7 +358,7 @@ private static unsafe void PrepareHandleAllowList(ProcessStartOptions options, I Interop.Kernel32.HandleOptions.HANDLE_FLAG_INHERIT, Interop.Kernel32.HandleOptions.HANDLE_FLAG_INHERIT)) { - throw new Win32Exception(Marshal.GetLastPInvokeError()); + throw new Win32Exception(); } } @@ -508,8 +515,7 @@ private void ResumeCore() int result = Interop.Kernel32.ResumeThread(_threadHandle); if (result == -1) { - int error = Marshal.GetLastPInvokeError(); - throw new Win32Exception(error); + throw new Win32Exception(); } } @@ -530,8 +536,7 @@ private void SendSignalCore(PosixSignal signal, bool entireProcessGroup) if (!Interop.Kernel32.GenerateConsoleCtrlEvent(ctrlEvent, ProcessId)) { - int error = Marshal.GetLastPInvokeError(); - throw new Win32Exception(error); + throw new Win32Exception(); } } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index 718c5403f429e9..c2411c20969c80 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -98,11 +98,7 @@ private static SafeProcessHandle StartInternal(ProcessStartOptions options, Safe if (input is null || output is null || error is null) { - nullHandle = File.OpenHandle( - OperatingSystem.IsWindows() ? "NUL" : "/dev/null", - FileMode.Open, - FileAccess.ReadWrite, - FileShare.ReadWrite); + nullHandle = File.OpenNullHandle(); input ??= nullHandle; output ??= nullHandle; diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 7b035657c71c32..413f8f8e5f133c 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -342,18 +342,6 @@ Invalid process handle. - - Failed to create job object. - - - Failed to set job object information. - - - Failed to get exit code of the process. - - - Process has not exited yet. - Cannot terminate entire process group because the process was not started with CreateNewProcessGroup=true. diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 81a7501305e36b..ff0e4c1ef07b30 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -231,10 +231,10 @@ Link="Common\Interop\Windows\Kernel32\Interop.JobObjects.cs" /> - - + + 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 cebd7469d43667..12fd12e7f71a25 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 @@ -861,7 +861,7 @@ private static void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHand } } - private static string GetEnvironmentVariablesBlock(DictionaryWrapper sd) + internal static string GetEnvironmentVariablesBlock(IDictionary sd) { // https://learn.microsoft.com/windows/win32/procthread/changing-environment-variables // "All strings in the environment block must be sorted alphabetically by name. The sort is diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs index e472e4109922c9..e61a44a19f8b5d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections.Generic; using System.Text; namespace System.Diagnostics @@ -26,32 +25,5 @@ internal static void BuildArgs(ProcessStartOptions options, ref ValueStringBuild } commandLine.NullTerminate(); } - - internal static string GetEnvironmentVariablesBlock(IDictionary sd) - { - // https://learn.microsoft.com/windows/win32/procthread/changing-environment-variables - // "All strings in the environment block must be sorted alphabetically by name. The sort is - // case-insensitive, Unicode order, without regard to locale. Because the equal sign is a - // separator, it must not be used in the name of an environment variable." - - var keys = new string[sd.Count]; - sd.Keys.CopyTo(keys, 0); - Array.Sort(keys, StringComparer.OrdinalIgnoreCase); - - // Join the null-terminated "key=val\0" strings - var result = new StringBuilder(8 * keys.Length); - foreach (string key in keys) - { - string? value = sd[key]; - - // Ignore null values for consistency with Environment.SetEnvironmentVariable - if (value != null) - { - result.Append(key).Append('=').Append(value).Append('\0'); - } - } - - return result.ToString(); - } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/Interop.cs b/src/libraries/System.Diagnostics.Process/tests/Interop.cs index 6bd2ddebbc8f6a..ce3f72ce74a693 100644 --- a/src/libraries/System.Diagnostics.Process/tests/Interop.cs +++ b/src/libraries/System.Diagnostics.Process/tests/Interop.cs @@ -73,10 +73,6 @@ public struct SID_AND_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); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs index 11992f29453dbe..76173add1f305a 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs @@ -31,7 +31,7 @@ private static void SendSignal(PosixSignal signal, int processId) _ => throw new ArgumentOutOfRangeException(nameof(signal)) }; - if (!Interop.GenerateConsoleCtrlEvent(dwCtrlEvent, (uint)processId)) + if (!Interop.Kernel32.GenerateConsoleCtrlEvent((int)dwCtrlEvent, processId)) { int error = Marshal.GetLastWin32Error(); if (error == Interop.Errors.ERROR_INVALID_FUNCTION && PlatformDetection.IsInContainer) diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs index c46543bad3d8a7..ca1595f00b074f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO; -using System.Runtime.InteropServices; using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; using Xunit; @@ -11,11 +10,6 @@ namespace System.Diagnostics.Tests { public partial class SafeProcessHandleTests { - [DllImport("kernel32.dll")] - private static extern IntPtr GetStdHandle(int nStdHandle); - - private const int STD_INPUT_HANDLE = -10; - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public void SendSignal_SIGINT_TerminatesProcessInNewProcessGroup() { @@ -30,7 +24,7 @@ public void SendSignal_SIGINT_TerminatesProcessInNewProcessGroup() CreateNewProcessGroup = true }; - using SafeFileHandle stdin = new(GetStdHandle(STD_INPUT_HANDLE), ownsHandle: false); + using SafeFileHandle stdin = Console.OpenStandardInputHandle(); using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: stdin, output: null, error: null); bool hasExited = processHandle.TryWaitForExit(TimeSpan.Zero, out _); @@ -57,7 +51,7 @@ public void Signal_SIGQUIT_TerminatesProcessInNewProcessGroup() CreateNewProcessGroup = true }; - using SafeFileHandle stdin = new(GetStdHandle(STD_INPUT_HANDLE), ownsHandle: false); + using SafeFileHandle stdin = Console.OpenStandardInputHandle(); using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: stdin, output: null, error: null); bool hasExited = processHandle.TryWaitForExit(TimeSpan.Zero, out _); @@ -84,7 +78,7 @@ public void Signal_UnsupportedSignal_ThrowsArgumentException() CreateNewProcessGroup = true }; - using SafeFileHandle stdin = new(GetStdHandle(STD_INPUT_HANDLE), ownsHandle: false); + using SafeFileHandle stdin = Console.OpenStandardInputHandle(); using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: stdin, output: null, error: null); try @@ -128,7 +122,7 @@ public void Kill_EntireProcessGroup_WithoutCreateNewProcessGroup_Throws() CreateNewProcessGroup = false }; - using SafeFileHandle stdin = new(GetStdHandle(STD_INPUT_HANDLE), ownsHandle: false); + using SafeFileHandle stdin = Console.OpenStandardInputHandle(); using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: stdin, output: null, error: null); Assert.Throws(() => processHandle.KillProcessGroup()); diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index 91e8dfef5b2a55..acc564f20d2986 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -14,6 +14,8 @@ namespace System.Diagnostics.Tests [PlatformSpecific(TestPlatforms.Windows)] public partial class SafeProcessHandleTests { + private static ProcessStartOptions CreateTenSecondSleep() => new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; + [Fact] public static void GetProcessId_ReturnsValidPid() { @@ -35,12 +37,10 @@ public static void GetProcessId_ReturnsValidPid() Assert.False(exitStatus.Canceled); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public static void Kill_KillsRunningProcess() { - ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(CreateTenSecondSleep(), input: null, output: null, error: null); bool wasKilled = processHandle.Kill(); Assert.True(wasKilled); @@ -50,12 +50,10 @@ public static void Kill_KillsRunningProcess() Assert.Equal(-1, exitStatus.ExitCode); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public static void Kill_CanBeCalledMultipleTimes() { - ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(CreateTenSecondSleep(), input: null, output: null, error: null); bool firstKill = processHandle.Kill(); Assert.True(firstKill); @@ -66,12 +64,10 @@ public static void Kill_CanBeCalledMultipleTimes() Assert.False(secondKill); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public static void WaitForExit_Called_After_Kill_ReturnsExitCodeImmediately() { - ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(CreateTenSecondSleep(), input: null, output: null, error: null); bool wasKilled = processHandle.Kill(); Assert.True(wasKilled); @@ -98,12 +94,10 @@ public static void Kill_OnAlreadyExitedProcess_ReturnsFalse() Assert.False(wasKilled); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public static void WaitForExitOrKillOnTimeout_KillsOnTimeout() { - ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(CreateTenSecondSleep(), input: null, output: null, error: null); Stopwatch stopwatch = Stopwatch.StartNew(); ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(300)); @@ -145,12 +139,10 @@ public static void TryWaitForExit_ReturnsTrueWhenProcessExitsBeforeTimeout() Assert.Null(exitStatus.Signal); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public static void TryWaitForExit_ReturnsFalseWhenProcessDoesNotExitBeforeTimeout() { - ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(CreateTenSecondSleep(), input: null, output: null, error: null); try { @@ -183,12 +175,10 @@ public static void WaitForExitOrKillOnTimeout_DoesNotKillWhenProcessExitsBeforeT Assert.Null(exitStatus.Signal); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public static void WaitForExitOrKillOnTimeout_KillsAndWaitsWhenTimeoutOccurs() { - ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(CreateTenSecondSleep(), input: null, output: null, error: null); Stopwatch stopwatch = Stopwatch.StartNew(); ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(300)); @@ -200,12 +190,10 @@ public static void WaitForExitOrKillOnTimeout_KillsAndWaitsWhenTimeoutOccurs() Assert.Equal(-1, exitStatus.ExitCode); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public static async Task WaitForExitOrKillOnCancellationAsync_KillsOnCancellation() { - ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 5" } }; - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(CreateTenSecondSleep(), input: null, output: null, error: null); Stopwatch stopwatch = Stopwatch.StartNew(); using CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(300)); @@ -217,12 +205,10 @@ public static async Task WaitForExitOrKillOnCancellationAsync_KillsOnCancellatio Assert.NotEqual(0, exitStatus.ExitCode); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))] public static async Task WaitForExitAsync_ThrowsOnCancellation() { - ProcessStartOptions options = new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 5" } }; - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(CreateTenSecondSleep(), input: null, output: null, error: null); try { From 2e5f8171fdf9298029362edde81ca0a578371d32 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 13 Feb 2026 16:53:21 +0100 Subject: [PATCH 26/33] fix the build --- .../tests/SafeProcessHandleTests.Windows.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs index ca1595f00b074f..37922549fc5e2b 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.Windows.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO; +using System.Runtime.InteropServices; using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; using Xunit; From e65c2bffe09354d41150741abf0371628fea9458 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 00:00:40 +0000 Subject: [PATCH 27/33] Fix bugs found during code review: remove CloseHandle on pseudo-handle, add wait after kill, validate timeout, remove dead code, use sizeof Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Windows.cs | 14 +++----------- .../Win32/SafeHandles/SafeProcessHandle.cs | 11 +++++++++-- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index a2aeff9aa686ab..c3da1d663dcef1 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -44,7 +44,7 @@ private SafeProcessHandle(IntPtr processHandle, IntPtr threadHandle, IntPtr proc ProcessId = processId; } - private static IntPtr CreateKillOnParentExitJob() + private static unsafe IntPtr CreateKillOnParentExitJob() { IntPtr jobHandle = Interop.Kernel32.CreateJobObjectW(IntPtr.Zero, IntPtr.Zero); if (jobHandle == IntPtr.Zero) @@ -59,7 +59,7 @@ private static IntPtr CreateKillOnParentExitJob() jobHandle, Interop.Kernel32.JOBOBJECTINFOCLASS.JobObjectExtendedLimitInformation, ref limitInfo, - (uint)Marshal.SizeOf())) + (uint)sizeof(Interop.Kernel32.JOBOBJECT_EXTENDED_LIMIT_INFORMATION))) { Interop.Kernel32.CloseHandle(jobHandle); throw new Win32Exception(); @@ -97,14 +97,6 @@ internal int GetExitCode() return exitCode; } - private bool TryGetExitCodeCore(out int exitCode, out PosixSignal? signal) - { - signal = default; - - return Interop.Kernel32.GetExitCodeProcess(this, out exitCode) - && exitCode != Interop.Kernel32.HandleOptions.STILL_ACTIVE; - } - private static unsafe SafeProcessHandle StartCore(ProcessStartOptions options, SafeFileHandle inputHandle, SafeFileHandle outputHandle, SafeFileHandle errorHandle, bool createSuspended) { Interop.Kernel32.STARTUPINFOEX startupInfoEx = default; @@ -297,7 +289,6 @@ ref processInfo Interop.Kernel32.DeleteProcThreadAttributeList(attributeList); Marshal.FreeHGlobal(attributeListBuffer); } - Interop.Kernel32.CloseHandle(currentProcHandle); } return procSH; @@ -396,6 +387,7 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) if (!processWaitHandle.WaitOne(milliseconds)) { wasKilledOnTimeout = KillCore(throwOnError: false); + processWaitHandle.WaitOne(Timeout.Infinite); } return new(GetExitCode(), wasKilledOnTimeout); diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index c2411c20969c80..ed0a7a03c761c9 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -302,7 +302,14 @@ private void Validate() } } - internal static int GetTimeoutInMilliseconds(TimeSpan timeout) => - timeout == Timeout.InfiniteTimeSpan ? Timeout.Infinite : (int)timeout.TotalMilliseconds; + internal static int GetTimeoutInMilliseconds(TimeSpan timeout) + { + long totalMilliseconds = (long)timeout.TotalMilliseconds; + + ArgumentOutOfRangeException.ThrowIfLessThan(totalMilliseconds, -1, nameof(timeout)); + ArgumentOutOfRangeException.ThrowIfGreaterThan(totalMilliseconds, int.MaxValue, nameof(timeout)); + + return (int)totalMilliseconds; + } } } From 5a266631b1a9e69dd6df060e2dd7900f43ae20ce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 08:19:19 +0000 Subject: [PATCH 28/33] Add ReaderWriterLockSlim to prevent accidental handle inheritance between Process.Start and SafeProcessHandle.Start Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Windows.cs | 13 +++++++++++++ .../src/System/Diagnostics/Process.Unix.cs | 1 - .../src/System/Diagnostics/Process.Windows.cs | 9 ++++++--- .../src/System/Diagnostics/Process.cs | 2 ++ 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index c3da1d663dcef1..4f8bf2cf96d0c7 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -98,6 +98,19 @@ internal int GetExitCode() } private static unsafe SafeProcessHandle StartCore(ProcessStartOptions options, SafeFileHandle inputHandle, SafeFileHandle outputHandle, SafeFileHandle errorHandle, bool createSuspended) + { + Process.s_processStartLock.EnterReadLock(); + try + { + return StartCoreSerialized(options, inputHandle, outputHandle, errorHandle, createSuspended); + } + finally + { + Process.s_processStartLock.ExitReadLock(); + } + } + + private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions options, SafeFileHandle inputHandle, SafeFileHandle outputHandle, SafeFileHandle errorHandle, bool createSuspended) { Interop.Kernel32.STARTUPINFOEX startupInfoEx = default; Interop.Kernel32.PROCESS_INFORMATION processInfo = default; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index fc46f14f918e7b..ba747427a5be91 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -18,7 +18,6 @@ public partial class Process : IDisposable { private static volatile bool s_initialized; private static readonly object s_initializedGate = new object(); - private static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim(); /// /// Puts a Process component in state to interact with operating system processes that run in a 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 12fd12e7f71a25..fda4b74afa7fff 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 @@ -16,8 +16,6 @@ namespace System.Diagnostics { public partial class Process : IDisposable { - private static readonly object s_createProcessLock = new object(); - private string? _processName; /// @@ -453,7 +451,8 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) // calls. We do not want one process to inherit the handles created concurrently for another // process, as that will impact the ownership and lifetimes of those handles now inherited // into multiple child processes. - lock (s_createProcessLock) + s_processStartLock.EnterWriteLock(); + try { try { @@ -630,6 +629,10 @@ ref processInfo // pointer to PROCESS_INFORMATION childErrorPipeHandle?.Dispose(); } } + finally + { + s_processStartLock.ExitWriteLock(); + } if (startInfo.RedirectStandardInput) { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 4218596c129644..768240e6958e63 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -24,6 +24,8 @@ namespace System.Diagnostics [Designer("System.Diagnostics.Design.ProcessDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class Process : Component { + internal static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim(); + private bool _haveProcessId; private int _processId; private bool _haveProcessHandle; From 180fc9bf25fcacdd4726eb656df1f1bd587881bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Feb 2026 09:35:50 +0000 Subject: [PATCH 29/33] Address jkotas feedback: simplify handle checks, fix OOM safety, fold catch into finally, rename interop file, fix coding conventions Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- ...eCtrlHandler.cs => Interop.ConsoleCtrl.cs} | 0 .../SafeHandles/SafeProcessHandle.Windows.cs | 64 +++++++++---------- .../src/System.Diagnostics.Process.csproj | 4 +- .../System.Diagnostics.Process.Tests.csproj | 4 +- 4 files changed, 36 insertions(+), 36 deletions(-) rename src/libraries/Common/src/Interop/Windows/Kernel32/{Interop.SetConsoleCtrlHandler.cs => Interop.ConsoleCtrl.cs} (100%) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetConsoleCtrlHandler.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ConsoleCtrl.cs similarity index 100% rename from src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetConsoleCtrlHandler.cs rename to src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ConsoleCtrl.cs diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index 4f8bf2cf96d0c7..fbec72d0955d4c 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -259,39 +259,21 @@ ref processInfo commandLine.Dispose(); } - if (processInfo.hProcess != IntPtr.Zero && processInfo.hProcess != new IntPtr(-1)) - { - if (createSuspended && processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) - { - procSH = new(processInfo.hProcess, processInfo.hThread, processGroupJobHandle, processInfo.dwProcessId); - } - else - { - procSH = new(processInfo.hProcess, IntPtr.Zero, processGroupJobHandle, processInfo.dwProcessId); - if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) - Interop.Kernel32.CloseHandle(processInfo.hThread); - } - } - else if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) - { - Interop.Kernel32.CloseHandle(processInfo.hThread); - } - - if (procSH is null) + if (errorCode != 0) { throw new Win32Exception(errorCode); } - } - catch - { - procSH?.Dispose(); - if (processGroupJobHandle != IntPtr.Zero) + procSH = new SafeProcessHandle( + processInfo.hProcess, + createSuspended ? processInfo.hThread : IntPtr.Zero, + processGroupJobHandle, + processInfo.dwProcessId); + + if (!createSuspended) { - Interop.Kernel32.CloseHandle(processGroupJobHandle); + Interop.Kernel32.CloseHandle(processInfo.hThread); } - - throw; } finally { @@ -302,6 +284,24 @@ ref processInfo Interop.Kernel32.DeleteProcThreadAttributeList(attributeList); Marshal.FreeHGlobal(attributeListBuffer); } + + if (procSH is null) + { + if (processInfo.hProcess != IntPtr.Zero) + { + Interop.Kernel32.CloseHandle(processInfo.hProcess); + } + + if (processInfo.hThread != IntPtr.Zero) + { + Interop.Kernel32.CloseHandle(processInfo.hThread); + } + + if (processGroupJobHandle != IntPtr.Zero) + { + Interop.Kernel32.CloseHandle(processGroupJobHandle); + } + } } return procSH; @@ -377,7 +377,7 @@ private ProcessExitStatus WaitForExitCore() using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); processWaitHandle.WaitOne(Timeout.Infinite); - return new(GetExitCode(), false); + return new ProcessExitStatus(GetExitCode(), false); } private bool TryWaitForExitCore(int milliseconds, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) @@ -389,7 +389,7 @@ private bool TryWaitForExitCore(int milliseconds, [NotNullWhen(true)] out Proces return false; } - exitStatus = new(GetExitCode(), false); + exitStatus = new ProcessExitStatus(GetExitCode(), false); return true; } @@ -403,7 +403,7 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) processWaitHandle.WaitOne(Timeout.Infinite); } - return new(GetExitCode(), wasKilledOnTimeout); + return new ProcessExitStatus(GetExitCode(), wasKilledOnTimeout); } private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) @@ -442,7 +442,7 @@ private async Task WaitForExitAsyncCore(CancellationToken can registeredWaitHandle?.Unregister(null); } - return new(GetExitCode(), false); + return new ProcessExitStatus(GetExitCode(), false); } private async Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) @@ -483,7 +483,7 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C registeredWaitHandle?.Unregister(null); } - return new(GetExitCode(), wasKilledBox.Value); + return new ProcessExitStatus(GetExitCode(), wasKilledBox.Value); } internal bool KillCore(bool throwOnError, bool entireProcessGroup = false) diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index ff0e4c1ef07b30..e415922131dfd0 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -233,8 +233,8 @@ Link="Common\Interop\Windows\Kernel32\Interop.ProcThreadAttributeList.cs" /> - + 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 b0d73ba06e6a3b..c64461f79c39ea 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 @@ -55,8 +55,8 @@ Link="Common\Interop\Windows\Kernel32\Interop.LoadLibrary.cs" /> - + From febd0a68ab8bdee76ba885721c6d57fe4a0cb0df Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 18 Feb 2026 11:52:50 +0100 Subject: [PATCH 30/33] cleanup --- .../Kernel32/Interop.GetWindowsDirectoryW.cs | 13 ------------- .../src/System.Diagnostics.Process.csproj | 3 --- 2 files changed, 16 deletions(-) delete mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs deleted file mode 100644 index 05710a47a019f9..00000000000000 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs +++ /dev/null @@ -1,13 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; - -internal static partial class Interop -{ - internal static partial class Kernel32 - { - [LibraryImport(Libraries.Kernel32, SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] - internal static partial uint GetWindowsDirectoryW(ref char lpBuffer, uint uSize); - } -} diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 1769ec2dc4a223..1ad9beb82fb3f6 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -12,7 +12,6 @@ $([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) SR.Process_PlatformNotSupported true - $(DefineConstants);WINDOWS @@ -142,8 +141,6 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetProcessPriorityBoost.cs" /> - Date: Wed, 18 Feb 2026 11:42:56 +0000 Subject: [PATCH 31/33] Address round 4 review feedback: Resume protection, NativeMemory.Alloc, move lock/env to ProcessUtils, refactor BuildArgs Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Unix.cs | 5 -- .../SafeHandles/SafeProcessHandle.Windows.cs | 47 ++++++++++++------- .../Win32/SafeHandles/SafeProcessHandle.cs | 13 ++++- ...ConfigureTerminalForChildProcesses.Unix.cs | 8 ++-- .../src/System/Diagnostics/Process.Unix.cs | 12 ++--- .../src/System/Diagnostics/Process.Windows.cs | 33 ++----------- .../src/System/Diagnostics/Process.cs | 1 - .../System/Diagnostics/ProcessStartOptions.cs | 2 + .../Diagnostics/ProcessUtils.Windows.cs | 43 ++++++++++++++--- .../src/System/Diagnostics/ProcessUtils.cs | 3 ++ 10 files changed, 95 insertions(+), 72 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 5f28b91bebac9c..1fe9f5434d25a5 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -32,11 +32,6 @@ internal SafeProcessHandle(int processId, SafeWaitHandle handle) : handle.DangerousAddRef(ref _releaseRef); } - /// - /// Gets the process ID. - /// - public int ProcessId { get; private set; } - protected override bool ReleaseHandle() { if (_releaseRef) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index fbec72d0955d4c..debddabbf9f0e4 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -30,11 +30,6 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali // This is specific to each process and is used to terminate the entire process group private IntPtr _processGroupJobHandle; - /// - /// Gets the process ID. - /// - public int ProcessId { get; private set; } - private SafeProcessHandle(IntPtr processHandle, IntPtr threadHandle, IntPtr processGroupJobHandle, int processId) : base(ownsHandle: true) { @@ -99,14 +94,14 @@ internal int GetExitCode() private static unsafe SafeProcessHandle StartCore(ProcessStartOptions options, SafeFileHandle inputHandle, SafeFileHandle outputHandle, SafeFileHandle errorHandle, bool createSuspended) { - Process.s_processStartLock.EnterReadLock(); + ProcessUtils.s_processStartLock.EnterReadLock(); try { return StartCoreSerialized(options, inputHandle, outputHandle, errorHandle, createSuspended); } finally { - Process.s_processStartLock.ExitReadLock(); + ProcessUtils.s_processStartLock.ExitReadLock(); } } @@ -120,6 +115,11 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions IntPtr attributeListBuffer = IntPtr.Zero; Interop.Kernel32.LPPROC_THREAD_ATTRIBUTE_LIST attributeList = default; + // In certain scenarios, the same handle may be passed for multiple stdio streams: + // - NUL file for all three + // - A single pipe/socket for both stdout and stderr (for combined output) + // - A single pipe/socket for stdin and stdout (for terminal emulation) + // - A single handle for all three streams using SafeFileHandle duplicatedInput = Duplicate(inputHandle, currentProcHandle); using SafeFileHandle duplicatedOutput = inputHandle.DangerousGetHandle() == outputHandle.DangerousGetHandle() ? duplicatedInput @@ -132,7 +132,7 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions int maxHandleCount = 3 + (options.HasInheritedHandlesBeenAccessed ? options.InheritedHandles.Count : 0); - IntPtr heapHandlesPtr = Marshal.AllocHGlobal(maxHandleCount * sizeof(IntPtr)); + IntPtr heapHandlesPtr = (IntPtr)NativeMemory.Alloc((nuint)maxHandleCount, (nuint)sizeof(IntPtr)); IntPtr* handlesToInherit = (IntPtr*)heapHandlesPtr; IntPtr processGroupJobHandle = IntPtr.Zero; @@ -148,6 +148,7 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions if (options.CreateNewProcessGroup) { + // This must happen before starting the process to ensure atomicity. processGroupJobHandle = Interop.Kernel32.CreateJobObjectW(IntPtr.Zero, IntPtr.Zero); if (processGroupJobHandle == IntPtr.Zero) { @@ -163,7 +164,7 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions Interop.Kernel32.LPPROC_THREAD_ATTRIBUTE_LIST emptyList = default; Interop.Kernel32.InitializeProcThreadAttributeList(emptyList, attributeCount, 0, ref size); - attributeListBuffer = Marshal.AllocHGlobal(size); + attributeListBuffer = (IntPtr)NativeMemory.Alloc((nuint)size); attributeList.AttributeList = attributeListBuffer; if (!Interop.Kernel32.InitializeProcThreadAttributeList(attributeList, attributeCount, 0, ref size)) @@ -221,7 +222,7 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions if (options.HasEnvironmentBeenAccessed) { creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT; - environmentBlock = Process.GetEnvironmentVariablesBlock(options.Environment); + environmentBlock = ProcessUtils.GetEnvironmentVariablesBlock(options.Environment); } string? workingDirectory = options.WorkingDirectory; @@ -231,7 +232,7 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions ValueStringBuilder commandLine = new(stackalloc char[256]); try { - ProcessUtils.BuildArgs(options, ref applicationName, ref commandLine); + ProcessUtils.BuildArgs(options.FileName, options.HasArgumentsBeenAccessed ? options.Arguments : null, ref applicationName, ref commandLine); fixed (char* environmentBlockPtr = environmentBlock) fixed (char* applicationNamePtr = &applicationName.GetPinnableReference()) @@ -277,12 +278,12 @@ ref processInfo } finally { - Marshal.FreeHGlobal(heapHandlesPtr); + NativeMemory.Free((void*)heapHandlesPtr); if (attributeListBuffer != IntPtr.Zero) { Interop.Kernel32.DeleteProcThreadAttributeList(attributeList); - Marshal.FreeHGlobal(attributeListBuffer); + NativeMemory.Free((void*)attributeListBuffer); } if (procSH is null) @@ -308,6 +309,10 @@ ref processInfo static SafeFileHandle Duplicate(SafeFileHandle sourceHandle, nint currentProcHandle) { + // From https://learn.microsoft.com/windows/win32/api/processthreadsapi/nf-processthreadsapi-updateprocthreadattribute: + // PROC_THREAD_ATTRIBUTE_HANDLE_LIST: "These handles must be created as inheritable handles and must not include pseudo handles". + // To ensure the handles we pass are inheritable, they are duplicated here. + if (!Interop.Kernel32.DuplicateHandle( currentProcHandle, sourceHandle, @@ -512,15 +517,23 @@ internal bool KillCore(bool throwOnError, bool entireProcessGroup = false) private void ResumeCore() { - if (_threadHandle == IntPtr.Zero) + IntPtr threadHandle = Interlocked.Exchange(ref _threadHandle, IntPtr.Zero); + if (threadHandle == IntPtr.Zero) { throw new InvalidOperationException(SR.CannotResumeNonSuspendedProcess); } - int result = Interop.Kernel32.ResumeThread(_threadHandle); - if (result == -1) + try { - throw new Win32Exception(); + int result = Interop.Kernel32.ResumeThread(threadHandle); + if (result == -1) + { + throw new Win32Exception(); + } + } + finally + { + Interop.Kernel32.CloseHandle(threadHandle); } } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index ed0a7a03c761c9..7308a8f5b87eec 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -19,6 +19,11 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali { internal static readonly SafeProcessHandle InvalidHandle = new SafeProcessHandle(); + /// + /// Gets the process ID. + /// + public int ProcessId { get; private set; } + /// /// Creates a . /// @@ -232,10 +237,14 @@ public bool KillProcessGroup() } /// - /// Resumes a suspended process. + /// Resumes a process that was created via . /// - /// Thrown when the handle is invalid. + /// Thrown when the process was not started in a suspended state, or has already been resumed. /// Thrown when the resume operation fails. + /// + /// This method can only be called once. After the process has been resumed, calling this method again will throw an . + /// This is not a general purpose process resume (like NtResumeProcess). It can only resume processes created via . + /// public void Resume() { Validate(); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs index f6746fad4efd4d..da7c8948cd0432 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.ConfigureTerminalForChildProcesses.Unix.cs @@ -17,7 +17,7 @@ internal static void ConfigureTerminalForChildProcesses(int increment, bool conf int childrenUsingTerminalRemaining = Interlocked.Add(ref s_childrenUsingTerminalCount, increment); if (increment > 0) { - Debug.Assert(s_processStartLock.IsReadLockHeld); + Debug.Assert(ProcessUtils.s_processStartLock.IsReadLockHeld); Debug.Assert(configureConsole); // At least one child is using the terminal. @@ -25,7 +25,7 @@ internal static void ConfigureTerminalForChildProcesses(int increment, bool conf } else { - Debug.Assert(s_processStartLock.IsWriteLockHeld); + Debug.Assert(ProcessUtils.s_processStartLock.IsWriteLockHeld); if (childrenUsingTerminalRemaining == 0 && configureConsole) { @@ -44,7 +44,7 @@ private static unsafe void SetDelayedSigChildConsoleConfigurationHandler() private static void DelayedSigChildConsoleConfiguration() { // Lock to avoid races with Process.Start - s_processStartLock.EnterWriteLock(); + ProcessUtils.s_processStartLock.EnterWriteLock(); try { if (s_childrenUsingTerminalCount == 0) @@ -55,7 +55,7 @@ private static void DelayedSigChildConsoleConfiguration() } finally { - s_processStartLock.ExitWriteLock(); + ProcessUtils.s_processStartLock.ExitWriteLock(); } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index dab31953a9db2a..943d88b53f51c3 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -500,7 +500,7 @@ private bool ForkAndExecProcess( // Lock to avoid races with OnSigChild // By using a ReaderWriterLock we allow multiple processes to start concurrently. - s_processStartLock.EnterReadLock(); + ProcessUtils.s_processStartLock.EnterReadLock(); try { if (usesTerminal) @@ -547,14 +547,14 @@ private bool ForkAndExecProcess( } finally { - s_processStartLock.ExitReadLock(); + ProcessUtils.s_processStartLock.ExitReadLock(); if (_waitStateHolder == null && usesTerminal) { // We failed to launch a child that could use the terminal. - s_processStartLock.EnterWriteLock(); + ProcessUtils.s_processStartLock.EnterWriteLock(); ConfigureTerminalForChildProcesses(-1); - s_processStartLock.ExitWriteLock(); + ProcessUtils.s_processStartLock.ExitWriteLock(); } } } @@ -1016,7 +1016,7 @@ private static int OnSigChild(int reapAll, int configureConsole) // DelayedSigChildConsoleConfiguration will be called. // Lock to avoid races with Process.Start - s_processStartLock.EnterWriteLock(); + ProcessUtils.s_processStartLock.EnterWriteLock(); try { bool childrenUsingTerminalPre = AreChildrenUsingTerminal; @@ -1028,7 +1028,7 @@ private static int OnSigChild(int reapAll, int configureConsole) } finally { - s_processStartLock.ExitWriteLock(); + ProcessUtils.s_processStartLock.ExitWriteLock(); } } 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 fda4b74afa7fff..f63d3e6d30e790 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 @@ -451,7 +451,7 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) // calls. We do not want one process to inherit the handles created concurrently for another // process, as that will impact the ownership and lifetimes of those handles now inherited // into multiple child processes. - s_processStartLock.EnterWriteLock(); + ProcessUtils.s_processStartLock.EnterWriteLock(); try { try @@ -511,7 +511,7 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) if (startInfo._environmentVariables != null) { creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT; - environmentBlock = GetEnvironmentVariablesBlock(startInfo._environmentVariables!); + environmentBlock = ProcessUtils.GetEnvironmentVariablesBlock(startInfo._environmentVariables!); } string? workingDirectory = startInfo.WorkingDirectory; @@ -631,7 +631,7 @@ ref processInfo // pointer to PROCESS_INFORMATION } finally { - s_processStartLock.ExitWriteLock(); + ProcessUtils.s_processStartLock.ExitWriteLock(); } if (startInfo.RedirectStandardInput) @@ -864,33 +864,6 @@ private static void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHand } } - internal static string GetEnvironmentVariablesBlock(IDictionary sd) - { - // https://learn.microsoft.com/windows/win32/procthread/changing-environment-variables - // "All strings in the environment block must be sorted alphabetically by name. The sort is - // case-insensitive, Unicode order, without regard to locale. Because the equal sign is a - // separator, it must not be used in the name of an environment variable." - - var keys = new string[sd.Count]; - sd.Keys.CopyTo(keys, 0); - Array.Sort(keys, StringComparer.OrdinalIgnoreCase); - - // Join the null-terminated "key=val\0" strings - var result = new StringBuilder(8 * keys.Length); - foreach (string key in keys) - { - string? value = sd[key]; - - // Ignore null values for consistency with Environment.SetEnvironmentVariable - if (value != null) - { - result.Append(key).Append('=').Append(value).Append('\0'); - } - } - - return result.ToString(); - } - private static string GetErrorMessage(int error) => Interop.Kernel32.GetMessage(error); /// Gets the friendly name of the process. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 768240e6958e63..ec540436fa8253 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -24,7 +24,6 @@ namespace System.Diagnostics [Designer("System.Diagnostics.Design.ProcessDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class Process : Component { - internal static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim(); private bool _haveProcessId; private int _processId; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index b74474dc06278b..6b58234b5132f8 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -138,6 +138,8 @@ public IList InheritedHandles internal bool HasInheritedHandlesBeenAccessed => _inheritedHandles is not null; + internal bool HasArgumentsBeenAccessed => _arguments is not null; + /// /// Initializes a new instance of the class. /// diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs index 7dd024e7121265..a28f4296ec6854 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.IO; using System.Text; @@ -8,25 +9,53 @@ namespace System.Diagnostics { internal static partial class ProcessUtils { - internal static void BuildArgs(ProcessStartOptions options, ref ValueStringBuilder applicationName, ref ValueStringBuilder commandLine) + internal static void BuildArgs(string resolvedFilePath, IList? arguments, ref ValueStringBuilder applicationName, ref ValueStringBuilder commandLine) { - string absolutePath = options.FileName; - - applicationName.Append(absolutePath); + applicationName.Append(resolvedFilePath); applicationName.NullTerminate(); // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "Because argv[0] is the module name, C programmers generally repeat the module name as the first token in the command line." // The truth is that some programs REQUIRE it (example: findstr). That is why we repeat it. - PasteArguments.AppendArgument(ref commandLine, options.FileName); + PasteArguments.AppendArgument(ref commandLine, resolvedFilePath); - foreach (string argument in options.Arguments) + if (arguments is not null) { - PasteArguments.AppendArgument(ref commandLine, argument); + foreach (string argument in arguments) + { + PasteArguments.AppendArgument(ref commandLine, argument); + } } commandLine.NullTerminate(); } + internal static string GetEnvironmentVariablesBlock(IDictionary sd) + { + // https://learn.microsoft.com/windows/win32/procthread/changing-environment-variables + // "All strings in the environment block must be sorted alphabetically by name. The sort is + // case-insensitive, Unicode order, without regard to locale. Because the equal sign is a + // separator, it must not be used in the name of an environment variable." + + var keys = new string[sd.Count]; + sd.Keys.CopyTo(keys, 0); + Array.Sort(keys, StringComparer.OrdinalIgnoreCase); + + // Join the null-terminated "key=val\0" strings + var result = new StringBuilder(8 * keys.Length); + foreach (string key in keys) + { + string? value = sd[key]; + + // Ignore null values for consistency with Environment.SetEnvironmentVariable + if (value != null) + { + result.Append(key).Append('=').Append(value).Append('\0'); + } + } + + return result.ToString(); + } + private static bool IsExecutable(string fullPath) { return File.Exists(fullPath); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs index c3ead1de0030bf..9753f0b7d6370e 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs @@ -2,11 +2,14 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO; +using System.Threading; namespace System.Diagnostics { internal static partial class ProcessUtils { + internal static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim(); + internal static string? FindProgramInPath(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); From 8f3c12e8db3cd07883a6709532bd3ce6c287fd6f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Feb 2026 12:04:55 +0000 Subject: [PATCH 32/33] Remove leftover blank line in Process.cs and add test for starting process without arguments Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.cs | 1 - .../tests/SafeProcessHandleTests.cs | 13 +++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index ec540436fa8253..4218596c129644 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -24,7 +24,6 @@ namespace System.Diagnostics [Designer("System.Diagnostics.Design.ProcessDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class Process : Component { - private bool _haveProcessId; private int _processId; private bool _haveProcessHandle; diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index acc564f20d2986..083671a2b091ba 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -16,6 +16,19 @@ public partial class SafeProcessHandleTests { private static ProcessStartOptions CreateTenSecondSleep() => new("powershell") { Arguments = { "-InputFormat", "None", "-Command", "Start-Sleep 10" } }; + [Fact] + public static void Start_WithNoArguments_Succeeds() + { + ProcessStartOptions options = new("hostname"); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(options, input: null, output: null, error: null); + + ProcessExitStatus exitStatus = processHandle.WaitForExit(); + Assert.Equal(0, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + [Fact] public static void GetProcessId_ReturnsValidPid() { From adeeb03b455c1c99d7107e41c162199afa5d0745 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Feb 2026 19:26:29 +0000 Subject: [PATCH 33/33] Use typed pointers instead of IntPtr for NativeMemory.Alloc results Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Windows.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index debddabbf9f0e4..6ee7468682ef11 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -112,7 +112,7 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = default; SafeProcessHandle? procSH = null; IntPtr currentProcHandle = Interop.Kernel32.GetCurrentProcess(); - IntPtr attributeListBuffer = IntPtr.Zero; + void* attributeListBuffer = null; Interop.Kernel32.LPPROC_THREAD_ATTRIBUTE_LIST attributeList = default; // In certain scenarios, the same handle may be passed for multiple stdio streams: @@ -132,8 +132,7 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions int maxHandleCount = 3 + (options.HasInheritedHandlesBeenAccessed ? options.InheritedHandles.Count : 0); - IntPtr heapHandlesPtr = (IntPtr)NativeMemory.Alloc((nuint)maxHandleCount, (nuint)sizeof(IntPtr)); - IntPtr* handlesToInherit = (IntPtr*)heapHandlesPtr; + IntPtr* handlesToInherit = (IntPtr*)NativeMemory.Alloc((nuint)maxHandleCount, (nuint)sizeof(IntPtr)); IntPtr processGroupJobHandle = IntPtr.Zero; try @@ -164,8 +163,8 @@ private static unsafe SafeProcessHandle StartCoreSerialized(ProcessStartOptions Interop.Kernel32.LPPROC_THREAD_ATTRIBUTE_LIST emptyList = default; Interop.Kernel32.InitializeProcThreadAttributeList(emptyList, attributeCount, 0, ref size); - attributeListBuffer = (IntPtr)NativeMemory.Alloc((nuint)size); - attributeList.AttributeList = attributeListBuffer; + attributeListBuffer = NativeMemory.Alloc((nuint)size); + attributeList.AttributeList = (IntPtr)attributeListBuffer; if (!Interop.Kernel32.InitializeProcThreadAttributeList(attributeList, attributeCount, 0, ref size)) { @@ -278,12 +277,12 @@ ref processInfo } finally { - NativeMemory.Free((void*)heapHandlesPtr); + NativeMemory.Free(handlesToInherit); - if (attributeListBuffer != IntPtr.Zero) + if (attributeListBuffer != null) { Interop.Kernel32.DeleteProcThreadAttributeList(attributeList); - NativeMemory.Free((void*)attributeListBuffer); + NativeMemory.Free(attributeListBuffer); } if (procSH is null)