From 6d47198303847409a900f57bbe84195b32375093 Mon Sep 17 00:00:00 2001 From: Ben Crist Date: Sun, 20 Nov 2022 12:20:14 -0600 Subject: [PATCH 1/2] return error.AlreadyTerminated from std.ChildProcess.kill when necessary --- lib/std/child_process.zig | 10 ++++++++-- lib/std/os.zig | 4 ++-- lib/std/os/windows.zig | 3 ++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 6a5fabc41f..645cb841e4 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -221,7 +221,10 @@ pub const ChildProcess = struct { return term; } - try windows.TerminateProcess(self.id, exit_code); + windows.TerminateProcess(self.id, exit_code) catch |err| switch (err) { + error.PermissionDenied => return error.AlreadyTerminated, + else => return err, + }; try self.waitUnwrappedWindows(); return self.term.?; } @@ -231,7 +234,10 @@ pub const ChildProcess = struct { self.cleanupStreams(); return term; } - try os.kill(self.id, os.SIG.TERM); + os.kill(self.id, os.SIG.TERM) catch |err| switch (err) { + error.ProcessNotFound => return error.AlreadyTerminated, + else => return err, + }; try self.waitUnwrapped(); return self.term.?; } diff --git a/lib/std/os.zig b/lib/std/os.zig index d6d4f596a1..90e738d775 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -639,14 +639,14 @@ pub fn raise(sig: u8) RaiseError!void { @compileError("std.os.raise unimplemented for this target"); } -pub const KillError = error{PermissionDenied} || UnexpectedError; +pub const KillError = error{ ProcessNotFound, PermissionDenied } || UnexpectedError; pub fn kill(pid: pid_t, sig: u8) KillError!void { switch (errno(system.kill(pid, sig))) { .SUCCESS => return, .INVAL => unreachable, // invalid signal .PERM => return error.PermissionDenied, - .SRCH => unreachable, // always a race condition + .SRCH => return error.ProcessNotFound, else => |err| return unexpectedErrno(err), } } diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index d38a63ef39..05b754de8d 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -1593,11 +1593,12 @@ pub fn GetModuleFileNameW(hModule: ?HMODULE, buf_ptr: [*]u16, buf_len: DWORD) Ge return buf_ptr[0..rc :0]; } -pub const TerminateProcessError = error{Unexpected}; +pub const TerminateProcessError = error{ PermissionDenied, Unexpected }; pub fn TerminateProcess(hProcess: HANDLE, uExitCode: UINT) TerminateProcessError!void { if (kernel32.TerminateProcess(hProcess, uExitCode) == 0) { switch (kernel32.GetLastError()) { + Win32Error.ACCESS_DENIED => return error.PermissionDenied, else => |err| return unexpectedError(err), } } From fb9376bd0499e2124616da1aeed7fe4e8bdd4f52 Mon Sep 17 00:00:00 2001 From: Ben Crist Date: Sun, 20 Nov 2022 17:41:33 -0600 Subject: [PATCH 2/2] Double check that child processes have really exited when TerminateProcess reports ACCESS_DENIED --- lib/std/child_process.zig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 645cb841e4..f8cb1be221 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -222,7 +222,15 @@ pub const ChildProcess = struct { } windows.TerminateProcess(self.id, exit_code) catch |err| switch (err) { - error.PermissionDenied => return error.AlreadyTerminated, + error.PermissionDenied => { + // Usually when TerminateProcess triggers a ACCESS_DENIED error, it + // indicates that the process has already exited, but there may be + // some rare edge cases where our process handle no longer has the + // PROCESS_TERMINATE access right, so let's do another check to make + // sure the process is really no longer running: + windows.WaitForSingleObjectEx(self.handle, 0, false) catch return err; + return error.AlreadyTerminated; + }, else => return err, }; try self.waitUnwrappedWindows();