-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement child process reaping. #10720
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
using System; | ||
using System.Runtime.InteropServices; | ||
using Microsoft.Win32.SafeHandles; | ||
|
||
namespace Microsoft.DotNet.Cli.Utils | ||
{ | ||
internal static class NativeMethods | ||
{ | ||
internal static class Windows | ||
{ | ||
internal enum JobObjectInfoClass : uint | ||
{ | ||
JobObjectExtendedLimitInformation = 9, | ||
} | ||
|
||
[Flags] | ||
internal enum JobObjectLimitFlags : uint | ||
{ | ||
JobObjectLimitKillOnJobClose = 0x2000, | ||
} | ||
|
||
[StructLayout(LayoutKind.Sequential)] | ||
internal struct JobObjectBasicLimitInformation | ||
{ | ||
public Int64 PerProcessUserTimeLimit; | ||
public Int64 PerJobUserTimeLimit; | ||
public JobObjectLimitFlags LimitFlags; | ||
public UIntPtr MinimumWorkingSetSize; | ||
public UIntPtr MaximumWorkingSetSize; | ||
public UInt32 ActiveProcessLimit; | ||
public UIntPtr Affinity; | ||
public UInt32 PriorityClass; | ||
public UInt32 SchedulingClass; | ||
} | ||
|
||
[StructLayout(LayoutKind.Sequential)] | ||
internal struct IoCounters | ||
{ | ||
public UInt64 ReadOperationCount; | ||
public UInt64 WriteOperationCount; | ||
public UInt64 OtherOperationCount; | ||
public UInt64 ReadTransferCount; | ||
public UInt64 WriteTransferCount; | ||
public UInt64 OtherTransferCount; | ||
} | ||
|
||
[StructLayout(LayoutKind.Sequential)] | ||
internal struct JobObjectExtendedLimitInformation | ||
{ | ||
public JobObjectBasicLimitInformation BasicLimitInformation; | ||
public IoCounters IoInfo; | ||
public UIntPtr ProcessMemoryLimit; | ||
public UIntPtr JobMemoryLimit; | ||
public UIntPtr PeakProcessMemoryUsed; | ||
public UIntPtr PeakJobMemoryUsed; | ||
} | ||
|
||
[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] | ||
internal static extern SafeWaitHandle CreateJobObjectW(IntPtr lpJobAttributes, string lpName); | ||
|
||
[DllImport("kernel32.dll", SetLastError = true)] | ||
internal static extern bool SetInformationJobObject(IntPtr hJob, JobObjectInfoClass jobObjectInformationClass, IntPtr lpJobObjectInformation, UInt32 cbJobObjectInformationLength); | ||
|
||
[DllImport("kernel32.dll", SetLastError = true)] | ||
internal static extern bool AssignProcessToJobObject(IntPtr hJob, IntPtr hProcess); | ||
} | ||
|
||
internal static class Posix | ||
{ | ||
[DllImport("libc", SetLastError = true)] | ||
internal static extern int getpgid(int pid); | ||
|
||
[DllImport("libc", SetLastError = true)] | ||
internal static extern int kill(int pid, int sig); | ||
|
||
internal const int SIGINT = 2; | ||
internal const int SIGTERM = 15; | ||
nguerrera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
using System; | ||
using System.ComponentModel; | ||
using System.Diagnostics; | ||
using System.Runtime.InteropServices; | ||
using Microsoft.DotNet.PlatformAbstractions; | ||
using Microsoft.Win32.SafeHandles; | ||
|
||
using RuntimeEnvironment = Microsoft.DotNet.PlatformAbstractions.RuntimeEnvironment; | ||
|
||
namespace Microsoft.DotNet.Cli.Utils | ||
{ | ||
/// <summary> | ||
/// Responsible for reaping a target process if the current process terminates. | ||
/// </summary> | ||
/// <remarks> | ||
/// On Windows, a job object will be used to ensure the termination of the target | ||
/// process (and its tree) even if the current process is rudely terminated. | ||
/// | ||
/// On POSIX systems, the reaper will handle SIGTERM and attempt to forward the | ||
/// signal to the target process only. | ||
/// | ||
/// The reaper also suppresses SIGINT in the current process to allow the target | ||
/// process to handle the signal. | ||
/// </remarks> | ||
internal class ProcessReaper : IDisposable | ||
peterhuene marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
/// <summary> | ||
/// Creates a new process reaper. | ||
/// </summary> | ||
/// <param name="process">The target process to reap if the current process terminates. The process must already be started.</param> | ||
public ProcessReaper(Process process) | ||
{ | ||
_process = process; | ||
|
||
if (RuntimeEnvironment.OperatingSystemPlatform == Platform.Windows) | ||
{ | ||
_job = AssignProcessToJobObject(_process.Handle); | ||
} | ||
else | ||
{ | ||
AppDomain.CurrentDomain.ProcessExit += HandleProcessExit; | ||
} | ||
|
||
Console.CancelKeyPress += HandleCancelKeyPress; | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
if (RuntimeEnvironment.OperatingSystemPlatform == Platform.Windows) | ||
{ | ||
if (_job != null) | ||
{ | ||
// Clear the kill on close flag because the child process terminated successfully | ||
// If this fails, then we have no choice but to terminate any remaining processes in the job | ||
SetKillOnJobClose(_job.DangerousGetHandle(), false); | ||
|
||
_job.Dispose(); | ||
_job = null; | ||
} | ||
} | ||
else | ||
{ | ||
AppDomain.CurrentDomain.ProcessExit -= HandleProcessExit; | ||
} | ||
|
||
Console.CancelKeyPress -= HandleCancelKeyPress; | ||
} | ||
|
||
private static void HandleCancelKeyPress(object sender, ConsoleCancelEventArgs e) | ||
{ | ||
// Ignore SIGINT/SIGQUIT so that the process can handle the signal | ||
e.Cancel = true; | ||
} | ||
|
||
private static SafeWaitHandle AssignProcessToJobObject(IntPtr process) | ||
{ | ||
var job = NativeMethods.Windows.CreateJobObjectW(IntPtr.Zero, null); | ||
if (job == null || job.IsInvalid) | ||
{ | ||
return null; | ||
} | ||
|
||
if (!SetKillOnJobClose(job.DangerousGetHandle(), true)) | ||
{ | ||
job.Dispose(); | ||
return null; | ||
} | ||
|
||
if (!NativeMethods.Windows.AssignProcessToJobObject(job.DangerousGetHandle(), process)) | ||
{ | ||
job.Dispose(); | ||
return null; | ||
} | ||
|
||
return job; | ||
} | ||
|
||
private void HandleProcessExit(object sender, EventArgs args) | ||
{ | ||
var currentPid = Process.GetCurrentProcess().Id; | ||
bool sendChildSIGTERM = true; | ||
|
||
// First, try to SIGTERM our process group | ||
// If the pgid is not the same as pid, then this process is not the root of the group | ||
if (NativeMethods.Posix.getpgid(currentPid) == currentPid) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no code that calls `setpgid. Why deal with this specifically? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this process is a child of another that doesn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering when this would be true, so my comment was more suggesting to remove the whole if block. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most shells There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, right. If I start dotnet from a bash script though, they are not the same. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So unfortunately As such, I think I'll remove the pg kill. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Setting it before calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't Or did you meant to The only consistent way to do it would to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is what I meant, so the child would inherit it from the parent.
It's more tricky than I thought. If the dotnet process would change its own pgid, it would no longer receive signals (e.g. SIGINT) from the console. So setpgid needs to occur between fork and exec of the child. And the parent needs to take care of propagating the signals to the children. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Plus, no managed code can safely run between fork and exec. If we ever need something there, we’d need API changes in the BCL. (Édit: I see that was already pointed out.) |
||
{ | ||
if (NativeMethods.Posix.kill(-currentPid, NativeMethods.Posix.SIGTERM) == 0) | ||
{ | ||
// Successfully sent the signal to the entire group; don't send again to child | ||
sendChildSIGTERM = false; | ||
} | ||
} | ||
|
||
if (sendChildSIGTERM && NativeMethods.Posix.kill(_process.Id, NativeMethods.Posix.SIGTERM) != 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you want to be safe for 'pid recycling', you can add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add that. Thanks! |
||
{ | ||
// Couldn't send the signal, don't wait | ||
return; | ||
} | ||
|
||
// If SIGTERM was ignored by the target, then we'll still wait | ||
_process.WaitForExit(); | ||
|
||
Environment.ExitCode = _process.ExitCode; | ||
} | ||
|
||
private static bool SetKillOnJobClose(IntPtr job, bool value) | ||
{ | ||
var information = new NativeMethods.Windows.JobObjectExtendedLimitInformation | ||
{ | ||
BasicLimitInformation = new NativeMethods.Windows.JobObjectBasicLimitInformation | ||
{ | ||
LimitFlags = (value ? NativeMethods.Windows.JobObjectLimitFlags.JobObjectLimitKillOnJobClose : 0) | ||
} | ||
}; | ||
|
||
var length = Marshal.SizeOf(typeof(NativeMethods.Windows.JobObjectExtendedLimitInformation)); | ||
var informationPtr = Marshal.AllocHGlobal(length); | ||
|
||
try | ||
{ | ||
Marshal.StructureToPtr(information, informationPtr, false); | ||
|
||
if (!NativeMethods.Windows.SetInformationJobObject( | ||
job, | ||
NativeMethods.Windows.JobObjectInfoClass.JobObjectExtendedLimitInformation, | ||
informationPtr, | ||
(uint)length)) | ||
{ | ||
return false; | ||
} | ||
|
||
return true; | ||
} | ||
finally | ||
{ | ||
Marshal.FreeHGlobal(informationPtr); | ||
} | ||
} | ||
|
||
private Process _process; | ||
private SafeWaitHandle _job; | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.