From 5fbd271744e2e842901f97dbd6d0c1f0d36ac4e8 Mon Sep 17 00:00:00 2001 From: asmichi Date: Thu, 9 Sep 2021 00:55:20 +0900 Subject: [PATCH] Use PROC_THREAD_ATTRIBUTE_JOB_LIST to atomically assign a process to a job on creation --- .../Interop/Windows/Kernel32.CreateProcess.cs | 1 - .../Interop/Windows/Kernel32.JobObject.cs | 5 ----- .../Kernel32.ProcThreadAttributeList.cs | 1 + .../Windows/ProcThreadAttributeList.cs | 15 +++++++++++++ .../WindowsChildProcessStateHelper.cs | 21 +++++-------------- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/ChildProcess/Interop/Windows/Kernel32.CreateProcess.cs b/src/ChildProcess/Interop/Windows/Kernel32.CreateProcess.cs index d250b15..1ff614c 100644 --- a/src/ChildProcess/Interop/Windows/Kernel32.CreateProcess.cs +++ b/src/ChildProcess/Interop/Windows/Kernel32.CreateProcess.cs @@ -12,7 +12,6 @@ namespace Asmichi.Interop.Windows internal static partial class Kernel32 { // Process Creation Flags - public const int CREATE_SUSPENDED = 0x00000004; public const int CREATE_UNICODE_ENVIRONMENT = 0x00000400; public const int EXTENDED_STARTUPINFO_PRESENT = 0x00080000; public const int CREATE_NO_WINDOW = 0x08000000; diff --git a/src/ChildProcess/Interop/Windows/Kernel32.JobObject.cs b/src/ChildProcess/Interop/Windows/Kernel32.JobObject.cs index 9100759..6fb220d 100644 --- a/src/ChildProcess/Interop/Windows/Kernel32.JobObject.cs +++ b/src/ChildProcess/Interop/Windows/Kernel32.JobObject.cs @@ -21,11 +21,6 @@ public static extern SafeJobObjectHandle CreateJobObject( [In] IntPtr lpJobAttributes, [In] char[]? lpName); - [DllImport(DllName, SetLastError = true)] - public static extern bool AssignProcessToJobObject( - [In] SafeJobObjectHandle hJob, - [In] SafeProcessHandle hProcess); - [DllImport(DllName, SetLastError = true)] public static extern unsafe bool SetInformationJobObject( [In] SafeJobObjectHandle hJob, diff --git a/src/ChildProcess/Interop/Windows/Kernel32.ProcThreadAttributeList.cs b/src/ChildProcess/Interop/Windows/Kernel32.ProcThreadAttributeList.cs index 36c3d8c..2f586b4 100644 --- a/src/ChildProcess/Interop/Windows/Kernel32.ProcThreadAttributeList.cs +++ b/src/ChildProcess/Interop/Windows/Kernel32.ProcThreadAttributeList.cs @@ -11,6 +11,7 @@ namespace Asmichi.Interop.Windows internal static partial class Kernel32 { public static readonly IntPtr PROC_THREAD_ATTRIBUTE_HANDLE_LIST = new IntPtr(0x20002); + public static readonly IntPtr PROC_THREAD_ATTRIBUTE_JOB_LIST = new IntPtr(0x2000d); public static readonly IntPtr PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE = new IntPtr(0x20016); [DllImport(DllName, SetLastError = true)] diff --git a/src/ChildProcess/Interop/Windows/ProcThreadAttributeList.cs b/src/ChildProcess/Interop/Windows/ProcThreadAttributeList.cs index 96f00bd..5f26b8d 100644 --- a/src/ChildProcess/Interop/Windows/ProcThreadAttributeList.cs +++ b/src/ChildProcess/Interop/Windows/ProcThreadAttributeList.cs @@ -39,6 +39,21 @@ public unsafe void UpdateHandleList(IntPtr* handles, int count) } } + public unsafe void UpdateJobList(IntPtr* handles, int count) + { + if (!Kernel32.UpdateProcThreadAttribute( + _unmanaged, + 0, + Kernel32.PROC_THREAD_ATTRIBUTE_JOB_LIST, + handles, + sizeof(IntPtr) * count, + IntPtr.Zero, + IntPtr.Zero)) + { + throw new Win32Exception(); + } + } + public unsafe void UpdatePseudoConsole(IntPtr hPC) { if (!Kernel32.UpdateProcThreadAttribute( diff --git a/src/ChildProcess/ProcessManagement/WindowsChildProcessStateHelper.cs b/src/ChildProcess/ProcessManagement/WindowsChildProcessStateHelper.cs index 653837d..3f0610a 100644 --- a/src/ChildProcess/ProcessManagement/WindowsChildProcessStateHelper.cs +++ b/src/ChildProcess/ProcessManagement/WindowsChildProcessStateHelper.cs @@ -63,22 +63,23 @@ public unsafe IChildProcessStateHolder SpawnProcess( var childStdOut = stdOut != null ? inheritableHandleStore.Add(stdOut) : null; var childStdErr = stdErr != null ? inheritableHandleStore.Add(stdErr) : null; + IntPtr jobObjectHandles = jobObjectHandle.DangerousGetHandle(); + Span inheritableHandles = stackalloc IntPtr[inheritableHandleStore.Count]; inheritableHandleStore.DangerousGetHandles(inheritableHandles); fixed (IntPtr* pInheritableHandles = inheritableHandles) { - using var attr = new ProcThreadAttributeList(2); + using var attr = new ProcThreadAttributeList(3); if (pseudoConsole is not null) { attr.UpdatePseudoConsole(pseudoConsole.Handle.DangerousGetHandle()); } attr.UpdateHandleList(pInheritableHandles, inheritableHandles.Length); + attr.UpdateJobList(&jobObjectHandles, 1); - // Create the process suspended so that it will not create a grandchild process before we assign it to the job object. const int CreationFlags = Kernel32.CREATE_UNICODE_ENVIRONMENT - | Kernel32.EXTENDED_STARTUPINFO_PRESENT - | Kernel32.CREATE_SUSPENDED; + | Kernel32.EXTENDED_STARTUPINFO_PRESENT; int processId; (processId, processHandle, threadHandle) = InvokeCreateProcess( @@ -91,18 +92,6 @@ public unsafe IChildProcessStateHolder SpawnProcess( childStdErr, attr); - if (!Kernel32.AssignProcessToJobObject(jobObjectHandle, processHandle)) - { - // Normally this will not fail... - throw new Win32Exception(); - } - - if (Kernel32.ResumeThread(threadHandle) == -1) - { - // Normally this will not fail... - throw new Win32Exception(); - } - return new WindowsChildProcessState(processId, processHandle, jobObjectHandle, pseudoConsole, startInfo.AllowSignal); } }