Remove some overhead from Process.Start (#44691)

- Avoid StringBuilder marshaling
- Let CreateProcess{WithLogonW} determine the current working directory
- Avoid creating SafeHandles for GetCurrentProcess()
- Avoid forcing ProcessStartInfo.ArgumentList into existence just to check if it contains anything
- Prefer using ProcessStartInfo.Arguments in Start(string, IEnumerable<string>) if there's only one string in the enumerable
- Use ValueStringBuilder instead of StringBuilder to build up arguments, so as to use stack space / pooled char[]s and avoid actually needing to produce strings.
- Avoid unnecessary SafeHandle for PROCESS_INFORMATION.hThread

src/libraries/Common/src/Interop/Windows/Advapi32/Interop.CreateProcessWithLogon.cs

@@ -12,18 +12,16 @@ internal partial class Interop
     internal partial class Advapi32
     {
         [DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, ExactSpelling = true, SetLastError = true, BestFitMapping = false, EntryPoint = "CreateProcessWithLogonW")]
-        internal static extern bool CreateProcessWithLogonW(
+        internal static extern unsafe bool CreateProcessWithLogonW(
             string userName,
             string domain,
             IntPtr password,
             LogonFlags logonFlags,
             string? appName,
-#pragma warning disable CA1838 // reasonable use of StringBuilder to build up a command line
-            [In] StringBuilder cmdLine,
-#pragma warning restore CA1838
+            char* cmdLine,
             int creationFlags,
             IntPtr environmentBlock,
-            string lpCurrentDirectory,
+            string? lpCurrentDirectory,
             ref Interop.Kernel32.STARTUPINFO lpStartupInfo,
             ref Interop.Kernel32.PROCESS_INFORMATION lpProcessInformation);
 

src/libraries/Common/src/Interop/Windows/Advapi32/Interop.OpenProcessToken.cs

@@ -2,13 +2,14 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Microsoft.Win32.SafeHandles;
+using System;
 using System.Runtime.InteropServices;
 
 internal partial class Interop
 {
     internal partial class Advapi32
     {
         [DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)]
-        internal static extern bool OpenProcessToken(SafeProcessHandle ProcessHandle, int DesiredAccess, out SafeTokenHandle TokenHandle);
+        internal static extern bool OpenProcessToken(IntPtr ProcessHandle, int DesiredAccess, out SafeTokenHandle TokenHandle);
     }
 }

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateProcess.cs

@@ -13,17 +13,15 @@ internal partial class Interop
     internal partial class Kernel32
     {
         [DllImport(Libraries.Kernel32, CharSet = CharSet.Unicode, SetLastError = true, BestFitMapping = false, EntryPoint = "CreateProcessW")]
-        internal static extern bool CreateProcess(
+        internal static extern unsafe bool CreateProcess(
             string? lpApplicationName,
-#pragma warning disable CA1838 // reasonable use of StringBuilder to build up a command line
-            [In] StringBuilder lpCommandLine,
-#pragma warning restore CA1838
+            char* lpCommandLine,
             ref SECURITY_ATTRIBUTES procSecAttrs,
             ref SECURITY_ATTRIBUTES threadSecAttrs,
             bool bInheritHandles,
             int dwCreationFlags,
             IntPtr lpEnvironment,
-            string lpCurrentDirectory,
+            string? lpCurrentDirectory,
             ref STARTUPINFO lpStartupInfo,
             ref PROCESS_INFORMATION lpProcessInformation
         );

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle.cs → src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeAccessTokenHandle.cs

@@ -9,7 +9,7 @@ internal static partial class Interop
 {
     internal static partial class Kernel32
     {
-        [DllImport(Interop.Libraries.Kernel32, SetLastError = true)]
+        [DllImport(Libraries.Kernel32, SetLastError = true)]
         internal static extern bool DuplicateHandle(
             IntPtr hSourceProcessHandle,
             IntPtr hSourceHandle,

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeFileHandle.cs

@@ -0,0 +1,23 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Win32.SafeHandles;
+using System;
+using System.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal static partial class Kernel32
+    {
+        [DllImport(Libraries.Kernel32, SetLastError = true)]
+        internal static extern bool DuplicateHandle(
+            IntPtr hSourceProcessHandle,
+            SafeHandle hSourceHandle,
+            IntPtr hTargetProcess,
+            out SafeFileHandle targetHandle,
+            int dwDesiredAccess,
+            bool bInheritHandle,
+            int dwOptions
+        );
+    }
+}

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_IntPtr.cs → src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafePipeHandle.cs

@@ -5,20 +5,18 @@
 using System;
 using System.Runtime.InteropServices;
 
-internal partial class Interop
+internal static partial class Interop
 {
-    internal partial class Kernel32
+    internal static partial class Kernel32
     {
         [DllImport(Libraries.Kernel32, SetLastError = true)]
-        [return: MarshalAs(UnmanagedType.Bool)]
         internal static extern bool DuplicateHandle(
             IntPtr hSourceProcessHandle,
-            SafePipeHandle hSourceHandle,
+            SafeHandle hSourceHandle,
             IntPtr hTargetProcessHandle,
             out SafePipeHandle lpTargetHandle,
             uint dwDesiredAccess,
-            [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle,
+            bool bInheritHandle,
             uint dwOptions);
-
     }
 }

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeWaitHandle.cs

@@ -0,0 +1,23 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Win32.SafeHandles;
+using System;
+using System.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal static partial class Kernel32
+    {
+        [DllImport(Libraries.Kernel32, SetLastError = true)]
+        internal static extern bool DuplicateHandle(
+            IntPtr hSourceProcessHandle,
+            SafeHandle hSourceHandle,
+            IntPtr hTargetProcess,
+            out SafeWaitHandle targetHandle,
+            int dwDesiredAccess,
+            bool bInheritHandle,
+            int dwOptions
+        );
+    }
+}

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcessWaitHandle.cs

@@ -3,6 +3,7 @@
 
 #nullable enable
 using Microsoft.Win32.SafeHandles;
+using System;
 using System.Runtime.InteropServices;
 using System.Threading;
 
@@ -14,20 +15,21 @@ internal sealed class ProcessWaitHandle : WaitHandle
         {
             internal ProcessWaitHandle(SafeProcessHandle processHandle)
             {
-                SafeWaitHandle? waitHandle;
-                SafeProcessHandle currentProcHandle = Interop.Kernel32.GetCurrentProcess();
-                bool succeeded = Interop.Kernel32.DuplicateHandle(
+                IntPtr currentProcHandle = GetCurrentProcess();
+                bool succeeded = DuplicateHandle(
                     currentProcHandle,
                     processHandle,
                     currentProcHandle,
-                    out waitHandle,
+                    out SafeWaitHandle waitHandle,
                     0,
                     false,
-                    Interop.Kernel32.HandleOptions.DUPLICATE_SAME_ACCESS);
+                    HandleOptions.DUPLICATE_SAME_ACCESS);
 
                 if (!succeeded)
                 {
-                    Marshal.ThrowExceptionForHR(Marshal.GetHRForLastWin32Error());
+                    int error = Marshal.GetHRForLastWin32Error();
+                    waitHandle.Dispose();
+                    Marshal.ThrowExceptionForHR(error);
                 }
 
                 this.SetSafeWaitHandle(waitHandle);

src/libraries/Common/tests/TestUtilities/System/AdminHelpers.cs

@@ -40,9 +40,8 @@ public static unsafe bool IsProcessElevated()
                 return(userId == 0);
             }
 
-            IntPtr processHandle = Interop.Kernel32.GetCurrentProcess();
             SafeAccessTokenHandle token;
-            if (!Interop.Advapi32.OpenProcessToken(processHandle, TokenAccessLevels.Read, out token))
+            if (!Interop.Advapi32.OpenProcessToken(Interop.Kernel32.GetCurrentProcess(), TokenAccessLevels.Read, out token))
             {
                 throw new Win32Exception(Marshal.GetLastWin32Error(), "Open process token failed");
             }

src/libraries/Common/tests/TestUtilities/TestUtilities.csproj

@@ -35,8 +35,8 @@
   </ItemGroup>
   <!-- Windows imports -->
   <ItemGroup>
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess_IntPtr.cs"
-             Link="Common\Interop\Windows\Kernel32\Interop.GetCurrentProcess_IntPtr.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs"
+             Link="Common\Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\NtDll\Interop.RtlGetVersion.cs"
              Link="Common\Interop\Windows\NtDll\Interop.RtlGetVersion.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.OpenProcessToken_SafeAccessTokenHandle.cs"

src/libraries/System.Diagnostics.PerformanceCounter/src/System.Diagnostics.PerformanceCounter.csproj

@@ -72,13 +72,13 @@
              Link="Common\Interop\Windows\Kernel32\Interop.Constants.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.CreateFileMapping.cs"
              Link="Common\Interop\Windows\Kernel32\Interop.CreateFileMapping.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle_SafeProcessHandle.cs"
-             Link="Common\Interop\Windows\kernel32\Interop.DuplicateHandle.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle_SafeWaitHandle.cs"
+             Link="Common\Interop\Windows\kernel32\Interop.DuplicateHandle_SafeWaitHandle.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FreeLibrary.cs"
              Link="Common\Interop\Windows\Kernel32\Interop.FreeLibrary.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetComputerName.cs"
              Link="Common\Interop\Windows\Kernel32\Interop.GetComputerName.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess_SafeProcessHandle.cs"
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs"
              Link="Common\Interop\Windows\kernel32\Interop.GetCurrentProcess.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcessId.cs"
              Link="Common\Interop\Windows\Kernel32\Interop.GetCurrentProcessId.cs" />
@@ -151,4 +151,4 @@
     <PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
     <ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.CompilerServices.Unsafe\src\System.Runtime.CompilerServices.Unsafe.ilproj" />
   </ItemGroup>
-</Project>
+</Project>

src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <DefineConstants>$(DefineConstants);FEATURE_REGISTRY</DefineConstants>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -40,7 +40,9 @@
     <Compile Include="$(CommonPath)System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs"
              Link="Common\System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs" />
     <Compile Include="$(CommonPath)System\Threading\Tasks\TaskTimeoutExtensions.cs"
-         Link="Common\System\Threading\Tasks\TaskTimeoutExtensions.cs" />
+             Link="Common\System\Threading\Tasks\TaskTimeoutExtensions.cs" />
+    <Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
+             Link="Common\System\Text\ValueStringBuilder.cs" />
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetsWindows)' == 'true'">
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.EnumProcessModules.cs"
@@ -99,7 +101,7 @@
              Link="Common\Interop\Windows\Kernel32\Interop.CreateProcess.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.TerminateProcess.cs"
              Link="Common\Interop\Windows\Kernel32\Interop.TerminateProcess.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess_SafeProcessHandle.cs"
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs"
              Link="Common\Interop\Windows\kernel32\Interop.GetCurrentProcess.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.OpenProcess.cs"
              Link="Common\Interop\Windows\Kernel32\Interop.OpenProcess.cs" />
@@ -143,8 +145,10 @@
              Link="Common\Interop\Windows\Kernel32\Interop.GetPriorityClass.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.SetPriorityClass.cs"
              Link="Common\Interop\Windows\Kernel32\Interop.SetPriorityClass.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle_SafeProcessHandle.cs"
-             Link="Common\Interop\Windows\kernel32\Interop.DuplicateHandle.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle_SafeFileHandle.cs"
+         Link="Common\Interop\Windows\kernel32\Interop.DuplicateHandle_SafeFileHandle.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle_SafeWaitHandle.cs"
+             Link="Common\Interop\Windows\kernel32\Interop.DuplicateHandle_SafeWaitHandle.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.ProcessWaitHandle.cs"
              Link="Common\Interop\Windows\Kernel32\Interop.ProcessWaitHandle.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.OpenProcessToken.cs"
@@ -222,8 +226,6 @@
     <Compile Include="System\Diagnostics\ProcessStartInfo.Unix.cs" />
     <Compile Include="System\Diagnostics\ProcessWaitHandle.Unix.cs" />
     <Compile Include="System\Diagnostics\ProcessWaitState.Unix.cs" />
-    <Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
-             Link="Common\System\Text\ValueStringBuilder.cs" />
     <Compile Include="$(CommonPath)System\IO\StringParser.cs"
              Link="Common\System\IO\StringParser.cs" />
     <Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs"

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs

@@ -556,7 +556,7 @@ private bool ForkAndExecProcess(
         private static string[] ParseArgv(ProcessStartInfo psi, string? resolvedExe = null, bool ignoreArguments = false)
         {
             if (string.IsNullOrEmpty(resolvedExe) &&
-                (ignoreArguments || (string.IsNullOrEmpty(psi.Arguments) && psi.ArgumentList.Count == 0)))
+                (ignoreArguments || (string.IsNullOrEmpty(psi.Arguments) && !psi.HasArgumentList)))
             {
                 return new string[] { psi.FileName };
             }
@@ -579,7 +579,7 @@ private static string[] ParseArgv(ProcessStartInfo psi, string? resolvedExe = nu
                 {
                     ParseArgumentsIntoList(psi.Arguments, argvList);
                 }
-                else
+                else if (psi.HasArgumentList)
                 {
                     argvList.AddRange(psi.ArgumentList);
                 }