diff --git a/docs/extensions/extensions.md b/docs/extensions/extensions.md index 3e11a7e..46d78ba 100644 --- a/docs/extensions/extensions.md +++ b/docs/extensions/extensions.md @@ -6,7 +6,6 @@ These extensions allow to alter the behavior of the Windows service in order to ## Available extensions * [Shared Directory Mapper](shared-directory-mapper.md) - Allows mapping shared drives before starting the executable -* [Runaway Process Killer](runaway-process-killer.md) - Termination of processes started by the previous runs of WinSW ## Developer guide diff --git a/docs/extensions/runaway-process-killer.md b/docs/extensions/runaway-process-killer.md deleted file mode 100644 index 780b426..0000000 --- a/docs/extensions/runaway-process-killer.md +++ /dev/null @@ -1,49 +0,0 @@ -# Runaway Process Killer extension - -In particular cases Windows service wrapper may leak the process after the service completion. -It happens when WinSW gets terminated without executing the shutdown logic. -Examples: force kill of the service process, .NET Runtime crash, missing permissions to kill processes or a bug in the logic. - -Such runaway processes may conflict with the service process once it restarts. -This extension allows preventing it by running the runaway process termination on startup before the executable gets started. - -Since: WinSW 2.0. - -## Usage - -The extension can be configured via the [XML configuration file](../xml-config-file.md). Configuration sample: - -```xml - - - sampleService - Sample service - This is a stub service. - %BASE%\sleep.bat - - - - - - - - %BASE%\pid.txt - - 5000 - - - -``` - -## Notes - -* The current implementation of the the extension checks only the root process (started executable) -* If the runaway process is detected the entire, the entire process tree gets terminated -* WinSW gives the runaway process a chance to the gracefully terminate. -If it does not do it within the timeout, the process will be force-killed. -* If the force kill fails, the WinSW startup continues with a warning. diff --git a/samples/sample-runaway-process-killer.xml b/samples/sample-runaway-process-killer.xml deleted file mode 100644 index cb9c159..0000000 --- a/samples/sample-runaway-process-killer.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - SERVICE_NAME - Jenkins Slave - This service runs a slave for Jenkins continuous integration system. - C:\Program Files\Java\jre7\bin\java.exe - -Xrs -jar "%BASE%\slave.jar" -jnlpUrl ... - - - - - - - %BASE%\pid.txt - - 5000 - - - diff --git a/src/WinSW.Core/Native/JobApis.cs b/src/WinSW.Core/Native/JobApis.cs new file mode 100644 index 0000000..fb5bea4 --- /dev/null +++ b/src/WinSW.Core/Native/JobApis.cs @@ -0,0 +1,67 @@ +#pragma warning disable SA1310 // Field names should not contain underscore + +using System; +using System.Runtime.InteropServices; + +namespace WinSW.Native +{ + internal static class JobApis + { + internal const uint JOB_OBJECT_LIMIT_BREAKAWAY_OK = 0x00000800; + internal const uint JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE = 0x00002000; + + [DllImport(Libraries.Kernel32, SetLastError = true)] + internal static extern bool AssignProcessToJobObject(IntPtr job, IntPtr process); + + [DllImport(Libraries.Kernel32, SetLastError = true, CharSet = CharSet.Unicode)] + internal static extern IntPtr CreateJobObjectW(IntPtr jobAttributes = default, string? name = null); + + [DllImport(Libraries.Kernel32, SetLastError = true)] + internal static extern bool SetInformationJobObject( + IntPtr job, + JOBOBJECTINFOCLASS jobObjectInformationClass, + in JOBOBJECT_EXTENDED_LIMIT_INFORMATION jobObjectInformation, + int jobObjectInformationLength); + + internal enum JOBOBJECTINFOCLASS + { + JobObjectExtendedLimitInformation = 9, + } + + [StructLayout(LayoutKind.Sequential)] + internal struct JOBOBJECT_BASIC_LIMIT_INFORMATION + { + public long PerProcessUserTimeLimit; + public long PerJobUserTimeLimit; + public uint LimitFlags; + public IntPtr MinimumWorkingSetSize; + public IntPtr MaximumWorkingSetSize; + public uint ActiveProcessLimit; + public UIntPtr Affinity; + public uint PriorityClass; + public uint SchedulingClass; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct IO_COUNTERS + { + public ulong ReadOperationCount; + public ulong WriteOperationCount; + public ulong OtherOperationCount; + public ulong ReadTransferCount; + public ulong WriteTransferCount; + public ulong OtherTransferCount; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct JOBOBJECT_EXTENDED_LIMIT_INFORMATION + { + public JOBOBJECT_BASIC_LIMIT_INFORMATION BasicLimitInformation; + public IO_COUNTERS IoInfo; + public IntPtr ProcessMemoryLimit; + public IntPtr JobMemoryLimit; + public IntPtr PeakProcessMemoryUsed; + public IntPtr PeakJobMemoryUsed; + } + } +} diff --git a/src/WinSW.Plugins/RunawayProcessKillerExtension.cs b/src/WinSW.Plugins/RunawayProcessKillerExtension.cs deleted file mode 100644 index a18ca14..0000000 --- a/src/WinSW.Plugins/RunawayProcessKillerExtension.cs +++ /dev/null @@ -1,464 +0,0 @@ -using System; -using System.Diagnostics; -using System.IO; -using System.Runtime.InteropServices; -using System.Text; -using System.Xml; -using log4net; -using WinSW.Extensions; -using WinSW.Util; -using static WinSW.Plugins.RunawayProcessKiller.RunawayProcessKillerExtension.Native; - -namespace WinSW.Plugins.RunawayProcessKiller -{ - public partial class RunawayProcessKillerExtension : AbstractWinSWExtension - { - /// - /// Absolute path to the PID file, which stores ID of the previously launched process. - /// - public string Pidfile { get; private set; } - - /// - /// Defines the process termination timeout in milliseconds. - /// This timeout will be applied multiple times for each child process. - /// - public TimeSpan StopTimeout { get; private set; } - - /// - /// If true, the runaway process will be checked for the WinSW environment variable before termination. - /// This option is not documented AND not supposed to be used by users. - /// - public bool CheckWinSWEnvironmentVariable { get; private set; } - - public override string DisplayName => "Runaway Process Killer"; - - private string ServiceId { get; set; } - - private static readonly ILog Logger = LogManager.GetLogger(typeof(RunawayProcessKillerExtension)); - -#pragma warning disable CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. - public RunawayProcessKillerExtension() -#pragma warning restore CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. - { - // Default initializer - } - -#pragma warning disable CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. - public RunawayProcessKillerExtension(string pidfile, int stopTimeoutMs = 5000, bool checkWinSWEnvironmentVariable = true) -#pragma warning restore CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. - { - this.Pidfile = pidfile; - this.StopTimeout = TimeSpan.FromMilliseconds(stopTimeoutMs); - this.CheckWinSWEnvironmentVariable = checkWinSWEnvironmentVariable; - } - - private static unsafe string? ReadEnvironmentVariable(IntPtr processHandle, string variable) - { - if (IntPtr.Size == sizeof(long)) - { - return SearchEnvironmentVariable( - processHandle, - variable, - GetEnvironmentAddress64(processHandle).ToInt64(), - (handle, address, buffer, size) => NtReadVirtualMemory(handle, new IntPtr(address), buffer, new IntPtr(size))); - } - - if (Is64BitOSWhen32BitProcess(Process.GetCurrentProcess().Handle) && !Is64BitOSWhen32BitProcess(processHandle)) - { - return SearchEnvironmentVariable( - processHandle, - variable, - GetEnvironmentAddressWow64(processHandle), - (handle, address, buffer, size) => NtWow64ReadVirtualMemory64(handle, address, buffer, size)); - } - - return SearchEnvironmentVariable( - processHandle, - variable, - GetEnvironmentAddress32(processHandle).ToInt64(), - (handle, address, buffer, size) => NtReadVirtualMemory(handle, new IntPtr(address), buffer, new IntPtr(size))); - } - - private static bool Is64BitOSWhen32BitProcess(IntPtr processHandle) => - IsWow64Process(processHandle, out int isWow64) != 0 && isWow64 != 0; - - private unsafe delegate int ReadMemoryCallback(IntPtr processHandle, long baseAddress, void* buffer, int bufferSize); - - private static unsafe string? SearchEnvironmentVariable(IntPtr processHandle, string variable, long address, ReadMemoryCallback reader) - { - const int BaseBufferSize = 0x1000; - string variableKey = '\0' + variable + '='; - string buffer = new string('\0', BaseBufferSize + variableKey.Length); - fixed (char* bufferPtr = buffer) - { - long startAddress = address; - for (; ; ) - { - int status = reader(processHandle, address, bufferPtr, buffer.Length * sizeof(char)); - int index = buffer.IndexOf("\0\0"); - if (index >= 0) - { - break; - } - - address += BaseBufferSize * sizeof(char); - } - - for (; ; ) - { - int variableIndex = buffer.IndexOf(variableKey); - if (variableIndex >= 0) - { - int valueStartIndex = variableIndex + variableKey.Length; - int valueEndIndex = buffer.IndexOf('\0', valueStartIndex); - string value = buffer.Substring(valueStartIndex, valueEndIndex - valueStartIndex); - return value; - } - - address -= BaseBufferSize * sizeof(char); - if (address < startAddress) - { - break; - } - - int status = reader(processHandle, address, bufferPtr, buffer.Length * sizeof(char)); - } - } - - return null; - } - - private static unsafe IntPtr GetEnvironmentAddress32(IntPtr processHandle) - { - _ = NtQueryInformationProcess( - processHandle, - PROCESSINFOCLASS.ProcessBasicInformation, - out PROCESS_BASIC_INFORMATION32 information, - sizeof(PROCESS_BASIC_INFORMATION32)); - - PEB32 peb; - _ = NtReadVirtualMemory(processHandle, new IntPtr(information.PebBaseAddress), &peb, new IntPtr(sizeof(PEB32))); - RTL_USER_PROCESS_PARAMETERS32 parameters; - _ = NtReadVirtualMemory(processHandle, new IntPtr(peb.ProcessParameters), ¶meters, new IntPtr(sizeof(RTL_USER_PROCESS_PARAMETERS32))); - return new IntPtr(parameters.Environment); - } - - private static unsafe IntPtr GetEnvironmentAddress64(IntPtr processHandle) - { - _ = NtQueryInformationProcess( - processHandle, - PROCESSINFOCLASS.ProcessBasicInformation, - out PROCESS_BASIC_INFORMATION64 information, - sizeof(PROCESS_BASIC_INFORMATION64)); - - PEB64 peb; - _ = NtReadVirtualMemory(processHandle, new IntPtr(information.PebBaseAddress), &peb, new IntPtr(sizeof(PEB64))); - RTL_USER_PROCESS_PARAMETERS64 parameters; - _ = NtReadVirtualMemory(processHandle, new IntPtr(peb.ProcessParameters), ¶meters, new IntPtr(sizeof(RTL_USER_PROCESS_PARAMETERS64))); - return new IntPtr(parameters.Environment); - } - - private static unsafe long GetEnvironmentAddressWow64(IntPtr processHandle) - { - _ = NtWow64QueryInformationProcess64( - processHandle, - PROCESSINFOCLASS.ProcessBasicInformation, - out PROCESS_BASIC_INFORMATION64 information, - sizeof(PROCESS_BASIC_INFORMATION64)); - - PEB64 peb; - _ = NtWow64ReadVirtualMemory64(processHandle, information.PebBaseAddress, &peb, sizeof(PEB64)); - RTL_USER_PROCESS_PARAMETERS64 parameters; - _ = NtWow64ReadVirtualMemory64(processHandle, peb.ProcessParameters, ¶meters, sizeof(RTL_USER_PROCESS_PARAMETERS64)); - return parameters.Environment; - } - - public override void Configure(XmlServiceConfig config, XmlNode node) - { - // We expect the upper logic to process any errors - // TODO: a better parser API for types would be useful - this.Pidfile = XmlHelper.SingleElement(node, "pidfile", false)!; - this.StopTimeout = TimeSpan.FromMilliseconds(int.Parse(XmlHelper.SingleElement(node, "stopTimeout", false)!)); - this.ServiceId = config.Id; - - // TODO: Consider making it documented - var checkWinSWEnvironmentVariable = XmlHelper.SingleElement(node, "checkWinSWEnvironmentVariable", true); - this.CheckWinSWEnvironmentVariable = checkWinSWEnvironmentVariable is null ? true : bool.Parse(checkWinSWEnvironmentVariable); - } - - /// - /// This method checks if the PID file is stored on the disk and then terminates runaway processes if they exist. - /// - /// Unused logger - public override void OnWrapperStarted() - { - // Read PID file from the disk - int pid; - if (File.Exists(this.Pidfile)) - { - string pidstring; - try - { - pidstring = File.ReadAllText(this.Pidfile); - } - catch (Exception ex) - { - Logger.Error("Cannot read PID file from " + this.Pidfile, ex); - return; - } - - try - { - pid = int.Parse(pidstring); - } - catch (FormatException e) - { - Logger.Error("Invalid PID file number in '" + this.Pidfile + "'. The runaway process won't be checked", e); - return; - } - } - else - { - Logger.Warn("The requested PID file '" + this.Pidfile + "' does not exist. The runaway process won't be checked"); - return; - } - - // Now check the process - Logger.DebugFormat("Checking the potentially runaway process with PID={0}", pid); - Process proc; - try - { - proc = Process.GetProcessById(pid); - } - catch (ArgumentException) - { - Logger.Debug("No runaway process with PID=" + pid + ". The process has been already stopped."); - return; - } - - // Ensure the process references the service - string expectedEnvVarName = WinSWSystem.EnvVarNameServiceId; - string? affiliatedServiceId = ReadEnvironmentVariable(proc.Handle, expectedEnvVarName); - if (affiliatedServiceId is null && this.CheckWinSWEnvironmentVariable) - { - Logger.Warn("The process " + pid + " has no " + expectedEnvVarName + " environment variable defined. " - + "The process has not been started by WinSW, hence it won't be terminated."); - - return; - } - - // Check the service ID value - if (this.CheckWinSWEnvironmentVariable && !this.ServiceId.Equals(affiliatedServiceId)) - { - Logger.Warn("The process " + pid + " has been started by Windows service with ID='" + affiliatedServiceId + "'. " - + "It is another service (current service id is '" + this.ServiceId + "'), hence the process won't be terminated."); - return; - } - - // Kill the runaway process - StringBuilder bldr = new StringBuilder("Stopping the runaway process (pid="); - bldr.Append(pid); - bldr.Append(") and its children. Environment was "); - if (!this.CheckWinSWEnvironmentVariable) - { - bldr.Append("not "); - } - - bldr.Append("checked, affiliated service ID: "); - bldr.Append(affiliatedServiceId ?? "undefined"); - bldr.Append(", process to kill: "); - bldr.Append(proc); - - Logger.Warn(bldr.ToString()); - proc.StopTree(this.StopTimeout); - } - - /// - /// Records the started process PID for the future use in OnStart() after the restart. - /// - public override void OnProcessStarted(Process process) - { - Logger.Info("Recording PID of the started process:" + process.Id + ". PID file destination is " + this.Pidfile); - try - { - File.WriteAllText(this.Pidfile, process.Id.ToString()); - } - catch (Exception ex) - { - Logger.Error("Cannot update the PID file " + this.Pidfile, ex); - } - } - - internal static class Native - { - private const string Kernel32 = "kernel32.dll"; - private const string NTDll = "ntdll.dll"; - - [DllImport(Kernel32)] - internal static extern int IsWow64Process(IntPtr hProcess, out int Wow64Process); - - [DllImport(NTDll)] - internal static extern int NtQueryInformationProcess( - IntPtr ProcessHandle, - PROCESSINFOCLASS ProcessInformationClass, - out PROCESS_BASIC_INFORMATION32 ProcessInformation, - int ProcessInformationLength, - IntPtr ReturnLength = default); - - [DllImport(NTDll)] - internal static extern int NtQueryInformationProcess( - IntPtr ProcessHandle, - PROCESSINFOCLASS ProcessInformationClass, - out PROCESS_BASIC_INFORMATION64 ProcessInformation, - int ProcessInformationLength, - IntPtr ReturnLength = default); - - [DllImport(NTDll)] - internal static extern unsafe int NtReadVirtualMemory( - IntPtr ProcessHandle, - IntPtr BaseAddress, - void* Buffer, - IntPtr BufferSize, - IntPtr NumberOfBytesRead = default); - - [DllImport(NTDll)] - internal static extern int NtWow64QueryInformationProcess64( - IntPtr ProcessHandle, - PROCESSINFOCLASS ProcessInformationClass, - out PROCESS_BASIC_INFORMATION64 ProcessInformation, - int ProcessInformationLength, - IntPtr ReturnLength = default); - - [DllImport(NTDll)] - internal static extern unsafe int NtWow64ReadVirtualMemory64( - IntPtr ProcessHandle, - long BaseAddress, - void* Buffer, - long BufferSize, - long NumberOfBytesRead = default); - - internal enum PROCESSINFOCLASS - { - ProcessBasicInformation = 0, - } - - [StructLayout(LayoutKind.Sequential)] - internal readonly struct MEMORY_BASIC_INFORMATION - { - public readonly IntPtr BaseAddress; - private readonly IntPtr AllocationBase; - private readonly uint AllocationProtect; - public readonly IntPtr RegionSize; - private readonly uint State; - private readonly uint Protect; - private readonly uint Type; - } - - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct PROCESS_BASIC_INFORMATION32 - { - private readonly int Reserved1; - public readonly int PebBaseAddress; - private fixed int Reserved2[2]; - private readonly uint UniqueProcessId; - private readonly int Reserved3; - } - - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct PROCESS_BASIC_INFORMATION64 - { - private readonly long Reserved1; - public readonly long PebBaseAddress; - private fixed long Reserved2[2]; - private readonly ulong UniqueProcessId; - private readonly long Reserved3; - } - - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct PEB32 - { - private fixed byte Reserved1[2]; - private readonly byte BeingDebugged; - private fixed byte Reserved2[1]; - private fixed int Reserved3[2]; - private readonly int Ldr; - public readonly int ProcessParameters; - private fixed int Reserved4[3]; - private readonly int AtlThunkSListPtr; - private readonly int Reserved5; - private readonly uint Reserved6; - private readonly int Reserved7; - private readonly uint Reserved8; - private readonly uint AtlThunkSListPtr32; - private fixed int Reserved9[45]; - private fixed byte Reserved10[96]; - private readonly int PostProcessInitRoutine; - private fixed byte Reserved11[128]; - private fixed int Reserved12[1]; - private readonly uint SessionId; - } - - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct PEB64 - { - private fixed byte Reserved1[2]; - private readonly byte BeingDebugged; - private fixed byte Reserved2[1]; - private fixed long Reserved3[2]; - private readonly long Ldr; - public readonly long ProcessParameters; - private fixed long Reserved4[3]; - private readonly long AtlThunkSListPtr; - private readonly long Reserved5; - private readonly uint Reserved6; - private readonly long Reserved7; - private readonly uint Reserved8; - private readonly uint AtlThunkSListPtr32; - private fixed long Reserved9[45]; - private fixed byte Reserved10[96]; - private readonly long PostProcessInitRoutine; - private fixed byte Reserved11[128]; - private fixed long Reserved12[1]; - private readonly uint SessionId; - } - - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct RTL_USER_PROCESS_PARAMETERS32 - { - private fixed byte Reserved1[16]; - private fixed int Reserved2[10]; - private readonly UNICODE_STRING32 ImagePathName; - private readonly UNICODE_STRING32 CommandLine; - - internal readonly int Environment; - } - - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct RTL_USER_PROCESS_PARAMETERS64 - { - private fixed byte Reserved1[16]; - private fixed long Reserved2[10]; - private readonly UNICODE_STRING64 ImagePathName; - private readonly UNICODE_STRING64 CommandLine; - - internal readonly long Environment; - } - - [StructLayout(LayoutKind.Sequential)] - internal readonly struct UNICODE_STRING32 - { - private readonly ushort Length; - private readonly ushort MaximumLength; - private readonly int Buffer; - } - - [StructLayout(LayoutKind.Sequential)] - internal readonly struct UNICODE_STRING64 - { - private readonly ushort Length; - private readonly ushort MaximumLength; - private readonly long Buffer; - } - } - } -} diff --git a/src/WinSW.Plugins/WinSW.Plugins.csproj b/src/WinSW.Plugins/WinSW.Plugins.csproj index 973f576..fe517a3 100644 --- a/src/WinSW.Plugins/WinSW.Plugins.csproj +++ b/src/WinSW.Plugins/WinSW.Plugins.csproj @@ -4,7 +4,6 @@ net461;netcoreapp3.1 latest enable - true diff --git a/src/WinSW.Tests/Extensions/RunawayProcessKillerTest.cs b/src/WinSW.Tests/Extensions/RunawayProcessKillerTest.cs deleted file mode 100644 index daee8b5..0000000 --- a/src/WinSW.Tests/Extensions/RunawayProcessKillerTest.cs +++ /dev/null @@ -1,125 +0,0 @@ -using System; -using System.Diagnostics; -using System.IO; -using WinSW.Extensions; -using WinSW.Plugins.RunawayProcessKiller; -using WinSW.Tests.Util; -using WinSW.Util; -using Xunit; -using Xunit.Abstractions; - -namespace WinSW.Tests.Extensions -{ - public class RunawayProcessKillerExtensionTest : ExtensionTestBase - { - private readonly XmlServiceConfig serviceConfig; - - private readonly string testExtension = GetExtensionClassNameWithAssembly(typeof(RunawayProcessKillerExtension)); - - private readonly ITestOutputHelper output; - - public RunawayProcessKillerExtensionTest(ITestOutputHelper output) - { - this.output = output; - - string seedXml = -$@" - SERVICE_NAME - Jenkins Slave - This service runs a slave for Jenkins continuous integration system. - C:\Program Files\Java\jre7\bin\java.exe - -Xrs -jar \""%BASE%\slave.jar\"" -jnlpUrl ... - - - - foo/bar/pid.txt - 5000 - - -"; - this.serviceConfig = XmlServiceConfig.FromXml(seedXml); - } - - [Fact] - public void LoadExtensions() - { - WinSWExtensionManager manager = new WinSWExtensionManager(this.serviceConfig); - manager.LoadExtensions(); - _ = Assert.Single(manager.Extensions); - - // Check the file is correct - var extension = manager.Extensions["killRunawayProcess"] as RunawayProcessKillerExtension; - Assert.NotNull(extension); - Assert.Equal("foo/bar/pid.txt", extension.Pidfile); - Assert.Equal(5000, extension.StopTimeout.TotalMilliseconds); - } - - [Fact] - public void StartStopExtension() - { - WinSWExtensionManager manager = new WinSWExtensionManager(this.serviceConfig); - manager.LoadExtensions(); - manager.FireOnWrapperStarted(); - manager.FireBeforeWrapperStopped(); - } - - internal void ShouldKillTheSpawnedProcess() - { - var winswId = "myAppWithRunaway"; - var extensionId = "runaway-process-killer"; - var tmpDir = FilesystemTestHelper.CreateTmpDirectory(); - try - { - // Spawn the test process - Process proc = new Process(); - var ps = proc.StartInfo; - ps.FileName = "cmd.exe"; - ps.Arguments = "/c pause"; - ps.UseShellExecute = false; - ps.RedirectStandardOutput = true; - ps.EnvironmentVariables[WinSWSystem.EnvVarNameServiceId] = winswId; - proc.Start(); - - try - { - // Generate extension and ensure that the roundtrip is correct - var pidfile = Path.Combine(tmpDir, "process.pid"); - var config = ConfigXmlBuilder.Create(this.output, id: winswId) - .WithRunawayProcessKiller(new RunawayProcessKillerExtension(pidfile), extensionId) - .ToServiceConfig(); - WinSWExtensionManager manager = new WinSWExtensionManager(config); - manager.LoadExtensions(); - var extension = manager.Extensions[extensionId] as RunawayProcessKillerExtension; - Assert.NotNull(extension); - Assert.Equal(pidfile, extension.Pidfile); - - // Inject PID - File.WriteAllText(pidfile, proc.Id.ToString()); - - // Try to terminate - Assert.False(proc.HasExited, "Process " + proc + " has exited before the RunawayProcessKiller extension invocation"); - _ = proc.StandardOutput.Read(); - extension.OnWrapperStarted(); - Assert.True(proc.HasExited, "Process " + proc + " should have been terminated by RunawayProcessKiller"); - } - finally - { - if (!proc.HasExited) - { - Console.Error.WriteLine("Test: Killing runaway process with ID=" + proc.Id); - proc.StopTree(TimeSpan.FromMilliseconds(100)); - if (!proc.HasExited) - { - // The test is failed here anyway, but we add additional diagnostics info - Console.Error.WriteLine("Test: ProcessHelper failed to properly terminate process with ID=" + proc.Id); - } - } - } - } - finally - { - Directory.Delete(tmpDir, true); - } - } - } -} diff --git a/src/WinSW.Tests/Util/ConfigXmlBuilder.cs b/src/WinSW.Tests/Util/ConfigXmlBuilder.cs index 202758c..dd942d4 100644 --- a/src/WinSW.Tests/Util/ConfigXmlBuilder.cs +++ b/src/WinSW.Tests/Util/ConfigXmlBuilder.cs @@ -1,6 +1,5 @@ using System.Collections.Generic; using System.Text; -using WinSW.Plugins.RunawayProcessKiller; using WinSW.Tests.Extensions; using Xunit.Abstractions; @@ -120,20 +119,6 @@ namespace WinSW.Tests.Util return this.WithRawEntry(string.Format("<{0}>{1}", tagName, value)); } - public ConfigXmlBuilder WithRunawayProcessKiller(RunawayProcessKillerExtension ext, string extensionId = "killRunawayProcess", bool enabled = true) - { - var fullyQualifiedExtensionName = ExtensionTestBase.GetExtensionClassNameWithAssembly(typeof(RunawayProcessKillerExtension)); - StringBuilder str = new StringBuilder(); - str.AppendFormat(" \n", new object[] { enabled, fullyQualifiedExtensionName, extensionId }); - str.AppendFormat(" {0}\n", ext.Pidfile); - str.AppendFormat(" {0}\n", ext.StopTimeout.TotalMilliseconds); - str.AppendFormat(" {0}\n", ext.CheckWinSWEnvironmentVariable); - str.Append(" \n"); - this.ExtensionXmls.Add(str.ToString()); - - return this; - } - public ConfigXmlBuilder WithDownload(Download download) { StringBuilder xml = new StringBuilder(); diff --git a/src/WinSW/WrapperService.cs b/src/WinSW/WrapperService.cs index 37801df..27b453a 100644 --- a/src/WinSW/WrapperService.cs +++ b/src/WinSW/WrapperService.cs @@ -29,7 +29,8 @@ namespace WinSW private readonly XmlServiceConfig config; private Process process = null!; - private volatile Process? poststartProcess; + private volatile Process? startingProcess; + private volatile Process? stoppingProcess; internal WinSWExtensionManager ExtensionManager { get; } @@ -314,29 +315,29 @@ namespace WinSW this.process = this.StartProcess(this.config.Executable, startArguments, this.OnMainProcessExited, executableLogHandler); this.ExtensionManager.FireOnProcessStarted(this.process); - try + string? poststartExecutable = this.config.PoststartExecutable; + if (poststartExecutable != null) { - string? poststartExecutable = this.config.PoststartExecutable; - if (poststartExecutable != null) + try { using Process process = StartProcessLocked(); this.WaitForProcessToExit(process); this.LogInfo($"Post-start process '{process.Format()}' exited with code {process.ExitCode}."); process.StopDescendants(additionalStopTimeout); - this.poststartProcess = null; + this.startingProcess = null; Process StartProcessLocked() { lock (this) { - return this.poststartProcess = this.StartProcess(poststartExecutable, this.config.PoststartArguments); + return this.startingProcess = this.StartProcess(poststartExecutable, this.config.PoststartArguments); } } } - } - catch (Exception e) - { - Log.Error(e); + catch (Exception e) + { + Log.Error(e); + } } } @@ -350,10 +351,11 @@ namespace WinSW { try { - using Process process = this.StartProcess(prestopExecutable, this.config.PrestopArguments); + using Process process = StartProcessLocked(prestopExecutable, this.config.PrestopArguments); this.WaitForProcessToExit(process); this.LogInfo($"Pre-stop process '{process.Format()}' exited with code {process.ExitCode}."); process.StopDescendants(additionalStopTimeout); + this.stoppingProcess = null; } catch (Exception e) { @@ -381,11 +383,12 @@ namespace WinSW try { // TODO: Redirect logging to Log4Net once https://github.com/kohsuke/winsw/pull/213 is integrated - Process stopProcess = this.StartProcess(stopExecutable, stopArguments); + using Process stopProcess = StartProcessLocked(stopExecutable, stopArguments); Log.Debug("WaitForProcessToExit " + this.process.Id + "+" + stopProcess.Id); this.WaitForProcessToExit(stopProcess); stopProcess.StopDescendants(additionalStopTimeout); + this.stoppingProcess = null; this.WaitForProcessToExit(this.process); this.process.StopDescendants(this.config.StopTimeout); @@ -397,17 +400,16 @@ namespace WinSW } } - this.poststartProcess?.StopTree(additionalStopTimeout); - string? poststopExecutable = this.config.PoststopExecutable; if (poststopExecutable != null) { try { - using Process process = this.StartProcess(poststopExecutable, this.config.PoststopArguments); + using Process process = StartProcessLocked(poststopExecutable, this.config.PoststopArguments); this.WaitForProcessToExit(process); this.LogInfo($"Post-stop process '{process.Format()}' exited with code {process.ExitCode}."); process.StopDescendants(additionalStopTimeout); + this.stoppingProcess = null; } catch (Exception e) { @@ -424,6 +426,14 @@ namespace WinSW } Log.Info("Finished " + this.config.Id); + + Process StartProcessLocked(string executable, string? arguments) + { + lock (this) + { + return this.stoppingProcess = this.StartProcess(executable, arguments); + } + } } private void WaitForProcessToExit(Process process) @@ -477,7 +487,7 @@ namespace WinSW if (this.orderlyShutdown) { - this.LogInfo($"Child process '{display}' terminated with code {process.ExitCode}."); + this.LogInfo($"Child process '{display}' terminated."); } else { @@ -487,7 +497,8 @@ namespace WinSW lock (this) { - this.poststartProcess?.StopTree(new TimeSpan(TimeSpan.TicksPerMillisecond)); + this.startingProcess?.StopTree(additionalStopTimeout); + this.stoppingProcess?.StopTree(additionalStopTimeout); } // if we finished orderly, report that to SCM.