Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProcessWatchdog: no longer fail on exit code 259 on Windows #450

Merged
merged 5 commits into from
Dec 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions rd-net/Lifetimes/Diagnostics/LogLog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,14 @@ public static List<LogLogRecord> StoredRecords
}
}

internal static void ClearStoredRecords()
{
lock (ourLock)
{
ourRecords.Clear();
}
}

#endregion


Expand Down
51 changes: 44 additions & 7 deletions rd-net/Lifetimes/Diagnostics/ProcessWatchdog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,23 @@ namespace JetBrains.Diagnostics
/// </summary>
[PublicAPI] public static class ProcessWatchdog
{
public class Options
{
public int Pid { get; }
public Lifetime Lifetime { get; }
public TimeSpan? GracefulShutdownPeriod { get; set; }
public Action? BeforeProcessKill { get; set; }
public Action? KillCurrentProcess { get; set; }

public Options(int pid, Lifetime lifetime)
{
Pid = pid;
Lifetime = lifetime;
}
}

private static readonly ILog ourLogger = Log.GetLog(nameof(ProcessWatchdog));
private const int DELAY_BEFORE_RETRY = 1000;
internal const int DELAY_BEFORE_RETRY = 1000;
private const int ERROR_INVALID_PARAMETER = 87;

public static void StartWatchdogForPidEnvironmentVariable(string envVarName, Action? beforeProcessKill = null)
Expand Down Expand Up @@ -45,10 +60,27 @@ public static void StartWatchdogForPid(int pid, Action? beforeProcessKill = null
StartWatchdogForPid(pid, Lifetime.Eternal, beforeProcessKill: beforeProcessKill);
}

public static void StartWatchdogForPid(int pid, Lifetime lifetime, TimeSpan? gracefulShutdownPeriod = null, Action? beforeProcessKill = null)
public static void StartWatchdogForPid(
int pid,
Lifetime lifetime,
TimeSpan? gracefulShutdownPeriod = null,
Action? beforeProcessKill = null) =>
StartWatchdogForPid(new Options(pid, lifetime)
{
GracefulShutdownPeriod = gracefulShutdownPeriod,
BeforeProcessKill = beforeProcessKill
});

public static void StartWatchdogForPid(Options options)
{
var pid = options.Pid;
var watchThread = new Thread(() =>
{
var lifetime = options.Lifetime;
var beforeProcessKill = options.BeforeProcessKill;
var gracefulShutdownPeriod = options.GracefulShutdownPeriod;
var killCurrentProcess = options.KillCurrentProcess;

ourLogger.Info($"Monitoring parent process PID:{pid}");

var useWinApi = true;
Expand Down Expand Up @@ -84,7 +116,10 @@ public static void StartWatchdogForPid(int pid, Lifetime lifetime, TimeSpan? gra
// ignored
}

Process.GetCurrentProcess().Kill();
if (killCurrentProcess != null)
killCurrentProcess();
else
Process.GetCurrentProcess().Kill();
return;
}

Expand Down Expand Up @@ -136,16 +171,18 @@ private static bool ProcessExists_Windows(int pid)
var handle = IntPtr.Zero;
try
{
handle = Kernel32.OpenProcess(ProcessAccessRights.PROCESS_QUERY_LIMITED_INFORMATION, false, pid);
handle = Kernel32.OpenProcess(
ProcessAccessRights.PROCESS_QUERY_LIMITED_INFORMATION | ProcessAccessRights.SYNCHRONIZE,
false,
pid);
if (handle == IntPtr.Zero)
{
var errorCode = Marshal.GetLastWin32Error();
return errorCode == ERROR_INVALID_PARAMETER ? false : throw new Win32Exception(errorCode); // ERROR_INVALID_PARAMETER means that process doesn't exist
}

return Kernel32.GetExitCodeProcess(handle, out var exitCode)
? exitCode == ProcessExitCode.STILL_ALIVE
: throw new Win32Exception();
var isTerminated = Kernel32.WaitForSingleObject(handle, 0u) == 0u;
return !isTerminated;
}
finally
{
Expand Down
9 changes: 3 additions & 6 deletions rd-net/Lifetimes/Interop/Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@ public static extern IntPtr OpenProcess(
[In] ProcessAccessRights dwDesiredAccess,
[In] bool bInheritHandle,
[In] int dwProcessId);

[DllImport(DllName, SetLastError = true)]
public static extern bool GetExitCodeProcess(
[In] IntPtr hProcess,
[Out] out ProcessExitCode lpExitCode
);

public static extern uint WaitForSingleObject(IntPtr hHandle, uint dwMilliseconds);

[DllImport(DllName, SetLastError = true)]
public static extern bool CloseHandle(
[In] IntPtr handle
Expand Down
100 changes: 100 additions & 0 deletions rd-net/Test.Lifetimes/Diagnostics/ProcessWatchdogTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
#if !NET35
using System;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Threading.Tasks;
using JetBrains.Core;
using JetBrains.Diagnostics;
using JetBrains.Lifetimes;
using JetBrains.Util;
using NUnit.Framework;

namespace Test.Lifetimes.Diagnostics;

public class ProcessWatchdogTest : LifetimesTestBase
{
[Test]
public Task TestWithSleepingProcess() => DoTest(StartSleepingProcess, true);

[Test]
public async Task TestWithProcessReturning259()
{
if (!RuntimeInfo.IsRunningUnderWindows) return;
await DoTest(() => GetTerminatedProcess(259), false);
}

private static Task DoTest(Func<Process> processCreator, bool assertAlive) => Lifetime.UsingAsync(async lt =>
{
var process = lt.Bracket(
processCreator,
p =>
{
if (!p.HasExited) p.Kill();
p.Dispose();
});

var tcs = new TaskCompletionSource<Unit>();
var options = new ProcessWatchdog.Options(process.Id, lt)
{
BeforeProcessKill = () => tcs.SetResult(Unit.Instance),
KillCurrentProcess = () => { }
};

ProcessWatchdog.StartWatchdogForPid(options);

var timeForReliableDetection = ProcessWatchdog.DELAY_BEFORE_RETRY * 2;
var task = tcs.Task;
if (assertAlive)
{
await Task.Delay(timeForReliableDetection, lt);
Assert.IsFalse(process.HasExited, "Process should not be exited.");
Assert.IsFalse(task.IsCompleted, "Watchdog should not be triggered.");
}

if (!process.HasExited) process.Kill();

if (await Task.WhenAny(task, Task.Delay(timeForReliableDetection, lt)) != task)
{
Assert.Fail($"Termination of process {process.Id} wasn't detected during the timeout.");
}

var exs = Assert.Throws<AggregateException>(TestLogger.ExceptionLogger.ThrowLoggedExceptions).InnerExceptions;
Assert.IsTrue(
exs.All(e => e.Message.Contains($"Parent process PID:{process.Id} has quit, killing ourselves via Process.Kill")),
$"No expected data in some of the exceptions: {string.Join("\n", exs.Select(e => e.Message))}");
});

private Process StartSleepingProcess()
{
var startInfo = RuntimeInfo.IsRunningUnderWindows
? new ProcessStartInfo("cmd.exe", "/c ping 127.0.0.1 -n 30")
: new ProcessStartInfo("sleep", "30");
startInfo.UseShellExecute = false;
startInfo.CreateNoWindow = true;
startInfo.RedirectStandardOutput = true;
startInfo.RedirectStandardError = true;

var logger = Log.GetLog<ProcessWatchdogTest>();
var process = Process.Start(startInfo)!;
process.ErrorDataReceived += (_, args) => logger.Warn($"[{process.Id}] {args.Data}");
process.OutputDataReceived += (_, args) => logger.Info($"[{process.Id}] {args.Data}");
process.Exited += (_, _) => logger.Info($"[{process.Id}] Exited with code: {process.ExitCode}");

return process;
}

private Process GetTerminatedProcess(int exitCode)
{
var process = RuntimeInfo.IsRunningUnderWindows
? Process.Start(new ProcessStartInfo("cmd.exe", $"/c exit {exitCode.ToString(CultureInfo.InvariantCulture)}")
{
WindowStyle = ProcessWindowStyle.Hidden
})
: Process.Start("/usr/bin/sh", $"-c \"exit {exitCode.ToString(CultureInfo.InvariantCulture)}\"");
process!.WaitForExit();
Assert.AreEqual(exitCode, process.ExitCode);
return process;
}
}
#endif
2 changes: 1 addition & 1 deletion rd-net/Test.Lifetimes/TestLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private void RecycleLogLog()
}
}

LogLog.StoredRecords.Clear();
LogLog.ClearStoredRecords();
}

[CanBeNull]
Expand Down
Loading