diff --git a/src/Common/src/Interop/Unix/System.Native/Interop.WaitId.cs b/src/Common/src/Interop/Unix/System.Native/Interop.WaitId.cs index a8a0df58a867..3bf9e82e1145 100644 --- a/src/Common/src/Interop/Unix/System.Native/Interop.WaitId.cs +++ b/src/Common/src/Interop/Unix/System.Native/Interop.WaitId.cs @@ -10,17 +10,14 @@ internal static partial class Interop internal static partial class Sys { /// - /// Waits for terminated child processes. + /// Returns the pid of a terminated child without reaping it. /// - /// The PID of a child process. -1 for any child. - /// The output exit status of the process - /// Tells the OS to leave the child waitable or reap it. /// /// 1) returns the process id of a terminated child process - /// 2) if no children are waiting, 0 is returned + /// 2) if no children are terminated, 0 is returned /// 3) on error, -1 is returned /// - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_WaitIdExitedNoHang", SetLastError = true)] - internal static extern int WaitIdExitedNoHang(int pid, out int exitCode, bool keepWaitable); + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_WaitIdAnyExitedNoHangNoWait", SetLastError = true)] + internal static extern int WaitIdAnyExitedNoHangNoWait(); } } diff --git a/src/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs b/src/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs new file mode 100644 index 000000000000..8292971d48e8 --- /dev/null +++ b/src/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Sys + { + /// + /// Reaps a terminated child. + /// + /// + /// 1) when a child is reaped, its process id is returned + /// 2) if pid is not a child or there are no unwaited-for children, -1 is returned (errno=ECHILD) + /// 3) if the child has not yet terminated, 0 is returned + /// 4) on error, -1 is returned. + /// + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_WaitPidExitedNoHang", SetLastError = true)] + internal static extern int WaitPidExitedNoHang(int pid, out int exitCode); + } +} diff --git a/src/Native/Unix/System.Native/pal_process.cpp b/src/Native/Unix/System.Native/pal_process.cpp index 0d627f4588b6..1790e76deaa1 100644 --- a/src/Native/Unix/System.Native/pal_process.cpp +++ b/src/Native/Unix/System.Native/pal_process.cpp @@ -450,34 +450,46 @@ extern "C" void SystemNative_SysLog(SysLogPriority priority, const char* message syslog(static_cast(priority), message, arg1); } -extern "C" int32_t SystemNative_WaitIdExitedNoHang(int32_t pid, int32_t* exitCode, int32_t keepWaitable) +extern "C" int32_t SystemNative_WaitIdAnyExitedNoHangNoWait() { - assert(exitCode != nullptr); - siginfo_t siginfo; int32_t result; - idtype_t idtype = pid == -1 ? P_ALL : P_PID; - int options = WEXITED | WNOHANG; - if (keepWaitable != 0) - { - options |= WNOWAIT; - } - while (CheckInterrupted(result = waitid(idtype, static_cast(pid), &siginfo, options))); - if (idtype == P_ALL && result == -1 && errno == ECHILD) + while (CheckInterrupted(result = waitid(P_ALL, 0, &siginfo, WEXITED | WNOHANG | WNOWAIT))); + if (result == -1 && errno == ECHILD) { + // The calling process has no existing unwaited-for child processes. result = 0; } else if (result == 0 && siginfo.si_signo == SIGCHLD) { - if (siginfo.si_code == CLD_EXITED) + result = siginfo.si_pid; + } + return result; +} + +extern "C" int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode) +{ + assert(exitCode != nullptr); + + int32_t result; + int status; + while (CheckInterrupted(result = waitpid(pid, &status, WNOHANG))); + if (result > 0) + { + if (WIFEXITED(status)) + { + // the child terminated normally. + *exitCode = WEXITSTATUS(status); + } + else if (WIFSIGNALED(status)) { - *exitCode = siginfo.si_status; + // child process was terminated by a signal. + *exitCode = 128 + WTERMSIG(status); } else { - *exitCode = 128 + siginfo.si_status; + assert(false); } - result = siginfo.si_pid; } return result; } diff --git a/src/Native/Unix/System.Native/pal_process.h b/src/Native/Unix/System.Native/pal_process.h index 55b30accb572..71155f0d1eb5 100644 --- a/src/Native/Unix/System.Native/pal_process.h +++ b/src/Native/Unix/System.Native/pal_process.h @@ -206,13 +206,23 @@ extern "C" int32_t SystemNative_GetSid(int32_t pid); extern "C" void SystemNative_SysLog(SysLogPriority priority, const char* message, const char* arg1); /** - * Waits for terminated child processes. + * Returns the pid of a terminated child without reaping it. * * 1) returns the process id of a terminated child process - * 2) if no children are waiting, 0 is returned + * 2) if no children are terminated, 0 is returned * 3) on error, -1 is returned */ -extern "C" int32_t SystemNative_WaitIdExitedNoHang(int32_t pid, int32_t* exitCode, int32_t keepWaitable); +extern "C" int32_t SystemNative_WaitIdAnyExitedNoHangNoWait(); + +/** + * Reaps a terminated child. + * + * 1) when a child is reaped, its process id is returned + * 2) if pid is not a child or there are no unwaited-for children, -1 is returned (errno=ECHILD) + * 3) if the child has not yet terminated, 0 is returned + * 4) on error, -1 is returned. + */ +extern "C" int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode); /** * Gets the configurable limit or variable for system path or file descriptor options. diff --git a/src/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index c8de23462b6d..87a772cbd427 100644 --- a/src/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -351,6 +351,9 @@ Common\Interop\Unix\Interop.WaitId.cs + + Common\Interop\Unix\Interop.WaitPid.cs + diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index f9ab4d64752d..bb5e79ca6901 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -527,7 +527,7 @@ private bool TryReapChild() // Try to get the state of the child process int exitCode; - int waitResult = Interop.Sys.WaitIdExitedNoHang(_processId, out exitCode, keepWaitable: false); + int waitResult = Interop.Sys.WaitPidExitedNoHang(_processId, out exitCode); if (waitResult == _processId) { @@ -563,8 +563,7 @@ internal static void CheckChildren(bool reapAll) do { // Find a process that terminated without reaping it yet. - int exitCode; - pid = Interop.Sys.WaitIdExitedNoHang(-1, out exitCode, keepWaitable: true); + pid = Interop.Sys.WaitIdAnyExitedNoHangNoWait(); if (pid > 0) { if (s_childProcessWaitStates.TryGetValue(pid, out ProcessWaitState pws)) @@ -638,7 +637,7 @@ internal static void CheckChildren(bool reapAll) do { int exitCode; - pid = Interop.Sys.WaitIdExitedNoHang(-1, out exitCode, keepWaitable: false); + pid = Interop.Sys.WaitPidExitedNoHang(-1, out exitCode); } while (pid > 0); } } diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index c8a652f2687a..226aa0e273de 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -387,6 +387,21 @@ public void TestStartWithNonExistingUserThrows() Assert.Throws(() => p.Start()); } + [Fact] + public void TestExitCodeKilledChild() + { + using (Process p = CreateProcessLong()) + { + p.Start(); + p.Kill(); + p.WaitForExit(); + + // SIGKILL may change per platform + const int SIGKILL = 9; // Linux, macOS, FreeBSD, ... + Assert.Equal(128 + SIGKILL, p.ExitCode); + } + } + /// /// Tests when running as a normal user and starting a new process as the same user /// works as expected.