diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index c54b1fac83e955..fe0bc580e0404e 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -1164,5 +1164,57 @@ public void ChildProcess_WithParentSignalHandler_CanReceiveSignals() Assert.Equal(RemotelyInvokable.SuccessExitCode, childHandle.Process.ExitCode); }); } + + // Repro attempt for https://github.com/dotnet/runtime/issues/131944: + // Process.Kill(entireProcessTree: true) can hang indefinitely on macOS. + // The two-phase KillTree (SIGSTOP the whole tree, then SIGKILL) opens a window in which a + // direct child is SIGSTOP'd. On macOS, waitid(P_ALL, WEXITED|WNOHANG|WNOWAIT) also reports + // SIGSTOP'd children, so the SIGCHLD handler's CheckChildren loop spins forever (waitpid + // WNOHANG never reaps a stopped child, WNOWAIT keeps it waitable) while holding + // s_childProcessWaitStates. Any concurrent Process construction (e.g. the tree enumeration in + // another Kill) then blocks on that lock, so the stopped child is never SIGKILL'd -> deadlock. + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.OSX)] + public void Kill_EntireProcessTree_Concurrent_DoesNotHang() + { + const int TreeCount = 8; + const int Iterations = 30; + + for (int iteration = 0; iteration < Iterations; iteration++) + { + var roots = new Process[TreeCount]; + for (int i = 0; i < TreeCount; i++) + { + // Each direct child spawns a grandchild and then blocks forever, producing a + // small process tree rooted at a direct child of this test host. + Process root = CreateProcess(() => + { + using Process grandChild = Process.Start("/bin/sleep", "1000"); + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + root.Start(); + roots[i] = root; + } + + // Give the grandchildren time to start so the trees are fully formed. + Thread.Sleep(500); + + var tasks = new Task[TreeCount]; + for (int i = 0; i < TreeCount; i++) + { + Process root = roots[i]; + tasks[i] = Task.Run(() => root.Kill(entireProcessTree: true)); + } + + bool completed = Task.WaitAll(tasks, TimeSpan.FromSeconds(60)); + Assert.True(completed, $"Kill(entireProcessTree: true) hung on iteration {iteration}."); + + foreach (Process root in roots) + { + Assert.True(root.WaitForExit(WaitInMS)); + } + } + } } } diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index c5af90e397287e..68bac75bd10798 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -1179,25 +1179,62 @@ void SystemNative_SysLog(SysLogPriority priority, const char* message, const cha int32_t SystemNative_WaitIdAnyExitedNoHangNoWait(void) { - siginfo_t siginfo; - memset(&siginfo, 0, sizeof(siginfo)); int32_t result; - while (CheckInterrupted(result = waitid(P_ALL, 0, &siginfo, WEXITED | WNOHANG | WNOWAIT))); - if (result == 0) + while (true) { - // When there are no waitable children and WNOHANG is specified, - // waitid may return zero with si_pid unchanged. - assert(siginfo.si_pid == 0 || // no waitable child - siginfo.si_signo == SIGCHLD); // waitable child + siginfo_t siginfo; + memset(&siginfo, 0, sizeof(siginfo)); + while (CheckInterrupted(result = waitid(P_ALL, 0, &siginfo, WEXITED | WNOHANG | WNOWAIT))); + if (result == 0) + { + // When there are no waitable children and WNOHANG is specified, + // waitid may return zero with si_pid unchanged. + assert(siginfo.si_pid == 0 || // no waitable child + siginfo.si_signo == SIGCHLD); // waitable child - result = siginfo.si_pid; - } - else if (errno == ECHILD) - { - // The calling process has no existing unwaited-for child processes. - result = 0; + if (siginfo.si_pid == 0) + { + // No waitable child. + return 0; + } + + // We requested WEXITED only, but some platforms (notably macOS) also report + // children that have stopped (SIGSTOP) or continued (SIGCONT). Because WNOWAIT + // was specified, such a notification is not consumed and would be returned again + // on every call, causing the SIGCHLD handler (CheckChildren) to spin indefinitely + // while a process tree is temporarily stopped by Process.Kill(entireProcessTree: true). + // Only report children that have actually exited. + if (siginfo.si_code == CLD_EXITED || + siginfo.si_code == CLD_KILLED || + siginfo.si_code == CLD_DUMPED) + { + return siginfo.si_pid; + } + + // Consume the stopped/continued notification so it isn't observed again, then keep + // looking for a child that has exited. This is a no-op on platforms that correctly + // honor WEXITED (e.g. Linux), where this branch is never reached. + siginfo_t drain; + memset(&drain, 0, sizeof(drain)); + while (CheckInterrupted(result = waitid(P_PID, (id_t)siginfo.si_pid, &drain, WSTOPPED | WCONTINUED | WNOHANG))); + if (result != 0) + { + // Unable to consume the notification (e.g. the child changed state concurrently). + // Avoid spinning: report no exited child for now. A real exit will be observed on + // a subsequent SIGCHLD. + return 0; + } + continue; + } + else if (errno == ECHILD) + { + // The calling process has no existing unwaited-for child processes. + return 0; + } + + // Unexpected error. + return result; } - return result; } int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t* terminatingSignal)