[release/2.1] Fix Process.ExitCode on mac for killed processes (#29407)

src/Common/src/Interop/Unix/System.Native/Interop.WaitId.cs

@@ -10,17 +10,14 @@ internal static partial class Interop
     internal static partial class Sys
     {        
         /// <summary>
-        /// Waits for terminated child processes.
+        /// Returns the pid of a terminated child without reaping it.
         /// </summary>
-        /// <param name="pid">The PID of a child process. -1 for any child.</param>
-        /// <param name="status">The output exit status of the process</param>
-        /// <param name="keepWaitable">Tells the OS to leave the child waitable or reap it.</param>
         /// <returns>
         /// 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
         /// </returns>
-        [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();
     }
 }

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
+    {
+        /// <summary>
+        /// Reaps a terminated child.
+        /// </summary>
+        /// <returns>
+        /// 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.
+        /// </returns>
+        [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_WaitPidExitedNoHang", SetLastError = true)]
+        internal static extern int WaitPidExitedNoHang(int pid, out int exitCode);
+    }
+}

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<int>(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<id_t>(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;
 }

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.

src/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj

@@ -351,6 +351,9 @@
     <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.WaitId.cs">
       <Link>Common\Interop\Unix\Interop.WaitId.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.WaitPid.cs">
+      <Link>Common\Interop\Unix\Interop.WaitPid.cs</Link>
+    </Compile>
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetsLinux)' == 'true'">
     <Compile Include="System\Diagnostics\Process.Linux.cs" />

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);
                 }
             }

src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs

@@ -387,6 +387,21 @@ public void TestStartWithNonExistingUserThrows()
             Assert.Throws<Win32Exception>(() => 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);
+            }
+        }
+
         /// <summary>
         /// Tests when running as a normal user and starting a new process as the same user
         /// works as expected.