Skip to content

Commit

Permalink
Remove some overhead from Process.Start
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
stephentoub committed Nov 17, 2020
1 parent 69d6127 commit 8b2bab4
Show file tree
Hide file tree
Showing 27 changed files with 140 additions and 165 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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
);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#nullable enable
using Microsoft.Win32.SafeHandles;
using System;
using System.Runtime.InteropServices;
using System.Threading;

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/tests/TestUtilities/TestUtilities.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -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>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<DefineConstants>$(DefineConstants);FEATURE_REGISTRY</DefineConstants>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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" />
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
Expand All @@ -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);
}
Expand Down
Loading

0 comments on commit 8b2bab4

Please sign in to comment.