Remove some overhead from Process.Start (#44691)
authorStephen Toub <stoub@microsoft.com>
Tue, 17 Nov 2020 22:48:19 +0000 (17:48 -0500)
committerGitHub <noreply@github.com>
Tue, 17 Nov 2020 22:48:19 +0000 (17:48 -0500)
- 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

27 files changed:
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.CreateProcessWithLogon.cs
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.OpenProcessToken.cs
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateProcess.cs
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeAccessTokenHandle.cs [moved from src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle.cs with 90% similarity]
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeFileHandle.cs [new file with mode: 0644]
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafePipeHandle.cs [moved from src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_IntPtr.cs with 71% similarity]
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeProcessHandle.cs [deleted file]
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeWaitHandle.cs [new file with mode: 0644]
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCurrentProcess.cs [moved from src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCurrentProcess_IntPtr.cs with 100% similarity]
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCurrentProcess_SafeProcessHandle.cs [deleted file]
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ProcessWaitHandle.cs
src/libraries/Common/tests/TestUtilities/System/AdminHelpers.cs
src/libraries/Common/tests/TestUtilities/TestUtilities.csproj
src/libraries/System.Diagnostics.PerformanceCounter/src/System.Diagnostics.PerformanceCounter.csproj
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj
src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
src/libraries/System.Private.CoreLib/src/System/PasteArguments.Unix.cs
src/libraries/System.Private.CoreLib/src/System/PasteArguments.Windows.cs
src/libraries/System.Private.CoreLib/src/System/PasteArguments.cs
src/libraries/System.Security.AccessControl/src/System.Security.AccessControl.csproj
src/libraries/System.Security.Principal.Windows/src/System.Security.Principal.Windows.csproj

index 61e28bd..da3f4c6 100644 (file)
@@ -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);
 
index 2fd2afb..7e8e225 100644 (file)
@@ -2,6 +2,7 @@
 // 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
@@ -9,6 +10,6 @@ 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);
     }
 }
index 95a17f1..2961693 100644 (file)
@@ -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
         );
@@ -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,
diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeFileHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeFileHandle.cs
new file mode 100644 (file)
index 0000000..fb54231
--- /dev/null
@@ -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
+        );
+    }
+}
@@ -5,20 +5,18 @@ using Microsoft.Win32.SafeHandles;
 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);
-
     }
 }
diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeProcessHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeProcessHandle.cs
deleted file mode 100644 (file)
index 7bf1c06..0000000
+++ /dev/null
@@ -1,35 +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 Microsoft.Win32.SafeHandles;
-using System;
-using System.Runtime.InteropServices;
-
-internal partial class Interop
-{
-    internal partial class Kernel32
-    {
-        [DllImport(Libraries.Kernel32, SetLastError = true, BestFitMapping = false)]
-        internal static extern bool DuplicateHandle(
-            SafeProcessHandle hSourceProcessHandle,
-            SafeHandle hSourceHandle,
-            SafeProcessHandle hTargetProcess,
-            out SafeFileHandle targetHandle,
-            int dwDesiredAccess,
-            bool bInheritHandle,
-            int dwOptions
-        );
-
-        [DllImport(Libraries.Kernel32, SetLastError = true, BestFitMapping = false)]
-        internal static extern bool DuplicateHandle(
-            SafeProcessHandle hSourceProcessHandle,
-            SafeHandle hSourceHandle,
-            SafeProcessHandle hTargetProcess,
-            out SafeWaitHandle targetHandle,
-            int dwDesiredAccess,
-            bool bInheritHandle,
-            int dwOptions
-        );
-
-    }
-}
diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeWaitHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeWaitHandle.cs
new file mode 100644 (file)
index 0000000..5749d82
--- /dev/null
@@ -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
+        );
+    }
+}
diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCurrentProcess_SafeProcessHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCurrentProcess_SafeProcessHandle.cs
deleted file mode 100644 (file)
index 36cb3da..0000000
+++ /dev/null
@@ -1,14 +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 Microsoft.Win32.SafeHandles;
-using System.Runtime.InteropServices;
-
-internal partial class Interop
-{
-    internal partial class Kernel32
-    {
-        [DllImport(Libraries.Kernel32, SetLastError = true)]
-        internal static extern SafeProcessHandle GetCurrentProcess();
-    }
-}
index d6f0827..134e366 100644 (file)
@@ -3,6 +3,7 @@
 
 #nullable enable
 using Microsoft.Win32.SafeHandles;
+using System;
 using System.Runtime.InteropServices;
 using System.Threading;
 
@@ -14,20 +15,21 @@ internal partial class Interop
         {
             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);
index e60459e..67c9a00 100644 (file)
@@ -40,9 +40,8 @@ namespace System
                 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");
             }
index 5943195..6e5f45f 100644 (file)
@@ -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"
index 45ca887..1499b5c 100644 (file)
              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" />
     <PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
     <ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.CompilerServices.Unsafe\src\System.Runtime.CompilerServices.Unsafe.ilproj" />
   </ItemGroup>
-</Project>
\ No newline at end of file
+</Project>
index 7799b1f..c7f5c57 100644 (file)
@@ -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"
              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" />
              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"
     <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"
index 6b2e44b..c24fcee 100644 (file)
@@ -556,7 +556,7 @@ namespace System.Diagnostics
         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 @@ namespace System.Diagnostics
                 {
                     ParseArgumentsIntoList(psi.Arguments, argvList);
                 }
-                else
+                else if (psi.HasArgumentList)
                 {
                     argvList.AddRange(psi.ArgumentList);
                 }
index 44323f9..e904dbb 100644 (file)
@@ -458,13 +458,13 @@ namespace System.Diagnostics
             //    * CreateProcess allows you to redirect all or none of the standard IO handles, so we use
             //      GetStdHandle for the handles that are not being redirected
 
-            StringBuilder commandLine = BuildCommandLine(startInfo);
+            var commandLine = new ValueStringBuilder(stackalloc char[256]);
+            BuildCommandLine(startInfo, ref commandLine);
 
             Interop.Kernel32.STARTUPINFO startupInfo = default;
             Interop.Kernel32.PROCESS_INFORMATION processInfo = default;
             Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = default;
             SafeProcessHandle procSH = new SafeProcessHandle();
-            SafeThreadHandle threadSH = new SafeThreadHandle();
 
             // handles used in parent process
             SafeFileHandle? parentInputPipeHandle = null;
@@ -532,9 +532,12 @@ namespace System.Diagnostics
                         creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT;
                         environmentBlock = GetEnvironmentVariablesBlock(startInfo._environmentVariables!);
                     }
-                    string workingDirectory = startInfo.WorkingDirectory;
+
+                    string? workingDirectory = startInfo.WorkingDirectory;
                     if (workingDirectory.Length == 0)
-                        workingDirectory = Directory.GetCurrentDirectory();
+                    {
+                        workingDirectory = null;
+                    }
 
                     bool retVal;
                     int errorCode = 0;
@@ -554,6 +557,7 @@ namespace System.Diagnostics
 
                         fixed (char* passwordInClearTextPtr = startInfo.PasswordInClearText ?? string.Empty)
                         fixed (char* environmentBlockPtr = environmentBlock)
+                        fixed (char* commandLinePtr = &commandLine.GetPinnableReference(terminate: true))
                         {
                             IntPtr passwordPtr = (startInfo.Password != null) ?
                                 Marshal.SecureStringToGlobalAllocUnicode(startInfo.Password) : IntPtr.Zero;
@@ -566,7 +570,7 @@ namespace System.Diagnostics
                                     (passwordPtr != IntPtr.Zero) ? passwordPtr : (IntPtr)passwordInClearTextPtr,
                                     logonFlags,
                                     null,            // we don't need this since all the info is in commandLine
-                                    commandLine,
+                                    commandLinePtr,
                                     creationFlags,
                                     (IntPtr)environmentBlockPtr,
                                     workingDirectory,
@@ -586,10 +590,11 @@ namespace System.Diagnostics
                     else
                     {
                         fixed (char* environmentBlockPtr = environmentBlock)
+                        fixed (char* commandLinePtr = &commandLine.GetPinnableReference(terminate: true))
                         {
                             retVal = Interop.Kernel32.CreateProcess(
                                 null,                // we don't need this since all the info is in commandLine
-                                commandLine,         // pointer to the command line string
+                                commandLinePtr,      // pointer to the command line string
                                 ref unused_SecAttrs, // address to process security attributes, we don't need to inherit the handle
                                 ref unused_SecAttrs, // address to thread security attributes.
                                 true,                // handle inheritance flag
@@ -607,7 +612,7 @@ namespace System.Diagnostics
                     if (processInfo.hProcess != IntPtr.Zero && processInfo.hProcess != new IntPtr(-1))
                         procSH.InitialSetHandle(processInfo.hProcess);
                     if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1))
-                        threadSH.InitialSetHandle(processInfo.hThread);
+                        Interop.Kernel32.CloseHandle(processInfo.hThread);
 
                     if (!retVal)
                     {
@@ -623,8 +628,6 @@ namespace System.Diagnostics
                     childInputPipeHandle?.Dispose();
                     childOutputPipeHandle?.Dispose();
                     childErrorPipeHandle?.Dispose();
-
-                    threadSH?.Dispose();
                 }
             }
 
@@ -645,6 +648,8 @@ namespace System.Diagnostics
                 _standardError = new StreamReader(new FileStream(parentErrorPipeHandle!, FileAccess.Read, 4096, false), enc, true, 4096);
             }
 
+            commandLine.Dispose();
+
             if (procSH.IsInvalid)
                 return false;
 
@@ -661,14 +666,13 @@ namespace System.Diagnostics
 
         private bool _signaled;
 
-        private static StringBuilder BuildCommandLine(ProcessStartInfo startInfo)
+        private static void BuildCommandLine(ProcessStartInfo startInfo, ref ValueStringBuilder commandLine)
         {
             // Construct a StringBuilder with the appropriate command line
             // to pass to CreateProcess.  If the filename isn't already
             // in quotes, we quote it here.  This prevents some security
             // problems (it specifies exactly which part of the string
             // is the file to execute).
-            StringBuilder commandLine = new StringBuilder();
             ReadOnlySpan<char> fileName = startInfo.FileName.AsSpan().Trim();
             bool fileNameIsQuoted = fileName.Length > 0 && fileName[0] == '\"' && fileName[fileName.Length - 1] == '\"';
             if (!fileNameIsQuoted)
@@ -683,9 +687,7 @@ namespace System.Diagnostics
                 commandLine.Append('"');
             }
 
-            startInfo.AppendArgumentsTo(commandLine);
-
-            return commandLine;
+            startInfo.AppendArgumentsTo(ref commandLine);
         }
 
         /// <summary>Gets timing information for the current process.</summary>
@@ -713,15 +715,13 @@ namespace System.Diagnostics
         private static unsafe void SetPrivilege(string privilegeName, int attrib)
         {
             // this is only a "pseudo handle" to the current process - no need to close it later
-            SafeProcessHandle processHandle = Interop.Kernel32.GetCurrentProcess();
-
             SafeTokenHandle? hToken = null;
 
             try
             {
                 // get the process token so we can adjust the privilege on it.  We DO need to
                 // close the token when we're done with it.
-                if (!Interop.Advapi32.OpenProcessToken(processHandle, Interop.Kernel32.HandleOptions.TOKEN_ADJUST_PRIVILEGES, out hToken))
+                if (!Interop.Advapi32.OpenProcessToken(Interop.Kernel32.GetCurrentProcess(), Interop.Kernel32.HandleOptions.TOKEN_ADJUST_PRIVILEGES, out hToken))
                 {
                     throw new Win32Exception();
                 }
@@ -760,7 +760,7 @@ namespace System.Diagnostics
         ///     Note that the handle we stored in current process object will have all access we need.
         /// </devdoc>
         /// <internalonly/>
-        private SafeProcessHandle GetProcessHandle(int access, bool throwIfExited)
+        private SafeProcessHandle GetProcessHandle(int access, bool throwIfExited = true)
         {
             if (_haveProcessHandle)
             {
@@ -773,10 +773,9 @@ namespace System.Diagnostics
                     {
                         if (waitHandle.WaitOne(0))
                         {
-                            if (_haveProcessId)
-                                throw new InvalidOperationException(SR.Format(SR.ProcessHasExited, _processId.ToString()));
-                            else
-                                throw new InvalidOperationException(SR.ProcessHasExitedNoId);
+                            throw new InvalidOperationException(_haveProcessId ?
+                                SR.Format(SR.ProcessHasExited, _processId.ToString()) :
+                                SR.ProcessHasExitedNoId);
                         }
                     }
                 }
@@ -789,8 +788,7 @@ namespace System.Diagnostics
             else
             {
                 EnsureState(State.HaveId | State.IsLocal);
-                SafeProcessHandle handle = SafeProcessHandle.InvalidHandle;
-                handle = ProcessManager.OpenProcess(_processId, access, throwIfExited);
+                SafeProcessHandle handle = ProcessManager.OpenProcess(_processId, access, throwIfExited);
                 if (throwIfExited && (access & Interop.Advapi32.ProcessOptions.PROCESS_QUERY_INFORMATION) != 0)
                 {
                     if (Interop.Kernel32.GetExitCodeProcess(handle, out _exitCode) && _exitCode != Interop.Kernel32.HandleOptions.STILL_ACTIVE)
@@ -802,16 +800,6 @@ namespace System.Diagnostics
             }
         }
 
-        /// <devdoc>
-        ///     Gets a short-term handle to the process, with the given access.  If a handle exists,
-        ///     then it is reused.  If the process has exited, it throws an exception.
-        /// </devdoc>
-        /// <internalonly/>
-        private SafeProcessHandle GetProcessHandle(int access)
-        {
-            return GetProcessHandle(access, true);
-        }
-
         private static void CreatePipeWithSecurityAttributes(out SafeFileHandle hReadPipe, out SafeFileHandle hWritePipe, ref Interop.Kernel32.SECURITY_ATTRIBUTES lpPipeAttributes, int nSize)
         {
             bool ret = Interop.Kernel32.CreatePipe(out hReadPipe, out hWritePipe, ref lpPipeAttributes, nSize);
@@ -852,7 +840,7 @@ namespace System.Diagnostics
                 // One potential theory is that child process can do something brain dead like
                 // closing the parent end of the pipe and there by getting into a blocking situation
                 // as parent will not be draining the pipe at the other end anymore.
-                SafeProcessHandle currentProcHandle = Interop.Kernel32.GetCurrentProcess();
+                IntPtr currentProcHandle = Interop.Kernel32.GetCurrentProcess();
                 if (!Interop.Kernel32.DuplicateHandle(currentProcHandle,
                                                      hTmp,
                                                      currentProcHandle,
index ab5e7be..53fd787 100644 (file)
@@ -1218,7 +1218,7 @@ namespace System.Diagnostics
             {
                 throw new InvalidOperationException(SR.StandardErrorEncodingNotAllowed);
             }
-            if (!string.IsNullOrEmpty(startInfo.Arguments) && startInfo.ArgumentList.Count > 0)
+            if (!string.IsNullOrEmpty(startInfo.Arguments) && startInfo.HasArgumentList)
             {
                 throw new InvalidOperationException(SR.ArgumentAndArgumentListInitialized);
             }
index 050afb5..6379245 100644 (file)
@@ -63,17 +63,9 @@ namespace System.Diagnostics
             set => _arguments = value;
         }
 
-        public Collection<string> ArgumentList
-        {
-            get
-            {
-                if (_argumentList == null)
-                {
-                    _argumentList = new Collection<string>();
-                }
-                return _argumentList;
-            }
-        }
+        public Collection<string> ArgumentList => _argumentList ??= new Collection<string>();
+
+        internal bool HasArgumentList => _argumentList is not null && _argumentList.Count != 0;
 
         public bool CreateNoWindow { get; set; }
 
@@ -176,25 +168,23 @@ namespace System.Diagnostics
 
         internal string BuildArguments()
         {
-            if (_argumentList == null || _argumentList.Count == 0)
+            if (HasArgumentList)
             {
-                return Arguments;
-            }
-            else
-            {
-                var stringBuilder = new StringBuilder();
-                AppendArgumentsTo(stringBuilder);
-                return stringBuilder.ToString();
+                var arguments = new ValueStringBuilder(stackalloc char[256]);
+                AppendArgumentsTo(ref arguments);
+                return arguments.ToString();
             }
+
+            return Arguments;
         }
 
-        internal void AppendArgumentsTo(StringBuilder stringBuilder)
+        internal void AppendArgumentsTo(ref ValueStringBuilder stringBuilder)
         {
             if (_argumentList != null && _argumentList.Count > 0)
             {
                 foreach (string argument in _argumentList)
                 {
-                    PasteArguments.AppendArgument(stringBuilder, argument);
+                    PasteArguments.AppendArgument(ref stringBuilder, argument);
                 }
             }
             else if (!string.IsNullOrEmpty(Arguments))
index 74794bd..e13a604 100644 (file)
@@ -15,6 +15,8 @@
              Link="Common\System\ShouldNotBeInvokedException.cs" />
     <Compile Include="$(CommonTestPath)Microsoft\Win32\TempRegistryKey.cs"
              Link="Common\Microsoft\Win32\TempRegistryKey.cs" />
+    <Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
+             Link="Common\System\Text\ValueStringBuilder.cs" />
     <Compile Include="DelegateSynchronizeInvoke.cs" />
     <Compile Include="Helpers.cs" />
     <Compile Include="Interop.cs" />
index b322b78..66f0da7 100644 (file)
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FileOperations.cs"
              Link="Common\Interop\Windows\Interop.FileOperations.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FileTypes.cs"
-             Link="Common\CoreLib\Interop\Windows\Interop.FileTypes.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess_IntPtr.cs"
+             Link="Common\Interop\Windows\Interop.FileTypes.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs"
              Link="Common\Interop\Windows\Interop.GetCurrentProcess.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle_IntPtr.cs"
-             Link="Common\Interop\Windows\Interop.DuplicateHandle_IntPtr.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle_SafePipeHandle.cs"
+             Link="Common\Interop\Windows\Interop.DuplicateHandle_SafePipeHandle.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetFileType_SafeHandle.cs"
-             Link="Common\CoreLib\Interop\Windows\Interop.GetFileType.cs" />
+             Link="Common\Interop\Windows\Interop.GetFileType.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.CreatePipe_SafePipeHandle.cs"
              Link="Common\Interop\Windows\Interop.CreatePipe_SafePipeHandle.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.ConnectNamedPipe.cs"
index b0ad2cf..db4ea9c 100644 (file)
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentDirectory.cs">
       <Link>Common\Interop\Windows\Kernel32\Interop.GetCurrentDirectory.cs</Link>
     </Compile>
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess_IntPtr.cs">
-      <Link>Common\Interop\Windows\Kernel32\Interop.GetCurrentProcess_IntPtr.cs</Link>
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs">
+      <Link>Common\Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs</Link>
     </Compile>
     <Compile Include="$(CommonPath)\Interop\Windows\Kernel32\Interop.GetCurrentProcessId.cs">
       <Link>Common\Interop\Windows\Kernel32\Interop.GetCurrentProcessId.cs</Link>
index d85004c..f4d8b7a 100644 (file)
@@ -14,10 +14,10 @@ namespace System
         /// </summary>
         internal static string Paste(IEnumerable<string> arguments, bool pasteFirstArgumentUsingArgV0Rules)
         {
-            var stringBuilder = new StringBuilder();
+            var stringBuilder = new ValueStringBuilder(stackalloc char[256]);
             foreach (string argument in arguments)
             {
-                AppendArgument(stringBuilder, argument);
+                AppendArgument(ref stringBuilder, argument);
             }
             return stringBuilder.ToString();
         }
index fb1394f..06f7e9d 100644 (file)
@@ -14,7 +14,7 @@ namespace System
         /// </summary>
         internal static string Paste(IEnumerable<string> arguments, bool pasteFirstArgumentUsingArgV0Rules)
         {
-            var stringBuilder = new StringBuilder();
+            var stringBuilder = new ValueStringBuilder(stackalloc char[256]);
 
             foreach (string argument in arguments)
             {
@@ -53,7 +53,7 @@ namespace System
                 }
                 else
                 {
-                    AppendArgument(stringBuilder, argument);
+                    AppendArgument(ref stringBuilder, argument);
                 }
             }
 
index 189d2f1..39008fc 100644 (file)
@@ -7,7 +7,7 @@ namespace System
 {
     internal static partial class PasteArguments
     {
-        internal static void AppendArgument(StringBuilder stringBuilder, string argument)
+        internal static void AppendArgument(ref ValueStringBuilder stringBuilder, string argument)
         {
             if (stringBuilder.Length != 0)
             {
index c4579f6..1273cc6 100644 (file)
@@ -67,8 +67,8 @@
              Link="Common\Interop\Interop.OpenThreadToken_SafeTokenHandle.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.OpenProcessToken_IntPtr.cs"
              Link="Common\Interop\Interop.OpenProcessToken_IntPtrs.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess_IntPtr.cs"
-             Link="Common\Interop\Interop.GetCurrentProcess_IntPtr.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs"
+             Link="Common\Interop\Interop.GetCurrentProcess.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.SetThreadToken.cs"
              Link="Common\Interop\Interop.SetThreadToken.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentThread.cs"
index a1f020b..1c263e2 100644 (file)
@@ -47,7 +47,7 @@
              Link="Common\Interop\Interop.LSAStructs.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\SspiCli\Interop.SECURITY_LOGON_SESSION_DATA.cs"
              Link="Common\Interop\Interop.SECURITY_LOGON_SESSION_DATA.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess_IntPtr.cs"
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs"
              Link="Common\Interop\Interop.GetCurrentProcess.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCurrentThread.cs"
              Link="Common\Interop\Interop.GetCurrentThread.cs" />
@@ -59,8 +59,8 @@
              Link="Common\Interop\Interop.GetTokenInformation.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.DuplicateTokenEx.cs"
              Link="Common\Interop\Interop.DuplicateTokenEx.cs" />
-    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle.cs"
-             Link="Common\Interop\Interop.DuplicateHandle.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DuplicateHandle_SafeAccessTokenHandle.cs"
+             Link="Common\Interop\Interop.DuplicateHandle_SafeAccessTokenHandle.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.CloseHandle.cs"
              Link="Common\Interop\Interop.CloseHandle.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\SspiCli\Interop.LsaGetLogonSessionData.cs"
     <Reference Include="System.Security.Principal" />
     <Reference Include="System.Threading" />
   </ItemGroup>
-</Project>
\ No newline at end of file
+</Project>