Repository navigation
Conversation
There was a problem hiding this comment.
Would this need adjustment as well?
There was a problem hiding this comment.
I think we should consider the same handling here.
I also observed the stop notification without WNOWAIT, as described in my main reply. If this approach looks appropriate to you, I'd be happy to extend the fix and add a focused regression test.
There was a problem hiding this comment.
@bitbemol Please go ahead rebase and update the handling here. Thanks for the fix!
There was a problem hiding this comment.
Thanks @iCharlesHu! Rebased onto main and updated the handling in _reap.
_peekIfExitedand_reapnow share one decoder,TerminationStatus.init?(ifTerminated:), which returnsnilfor anything that isn't a termination (includingCLD_STOPPED).init(_ siginfo:)keeps itsfatalErrorfor genuinely unexpected codes.- I also fixed
_blockingReap, slightly beyond what you asked: a stop signal racing the child's exit can make the blockingwaitid(WEXITED)returnCLD_STOPPEDeven afterNOTE_EXIT, which trapped. It now retrieswaitidafter aCLD_STOPPEDresult; each stop is reported only once, so it can't spin. Happy to split this into a separate PR if you prefer. - Added regression tests for
_reap,_blockingReap(including a check that it doesn't spin), and teardown of a stopped child.
Root cause, for the open question in the description: XNU is built with __DARWIN_UNIX03 set to 0, so inside the kernel WSTOPPED expands to _WSTOPPED (0x7F) instead of 0x08, and waitid's options & WSTOPPED check matches any valid flags. Confirmed on macOS 27.0.1: an undefined flag 0x40 is accepted, and a WCONTINUED-only query reports CLD_STOPPED. waitpid isn't affected. This seems worth a Feedback report to Apple.
One note: reap() has had no callers in Sources since #272, so its fix is defensive. Let me know if you'd rather remove it instead.
|
Would you be able to provide some additional context on the change? e.g. what are some cases where si_pid and si_signo both being zero are not equivalent to the proposed condition? |
Thanks for reviewing this, @jakepetroules, and sorry for leaving the description empty! I've updated it with the reproduction details and test results. 😅 On macOS 27 beta ( That is where the checks differ: the original condition reports that the child has exited because the PID and signal are nonzero, while the proposed condition correctly rejects the nonterminal status. Reporting an exit prematurely can bypass monitoring and lead to the stop notification reaching I confirmed the unexpected Regarding your Thanks for your guidance! |
|
I'd also like @FranzBusch and/or @weissi to take a look if possible. |
Darwin's waitid reports a stopped child even when only WEXITED is requested, because the kernel's internal WSTOPPED is 0x7F. _reap decoded that CLD_STOPPED result and trapped, and _blockingReap could trap when a stop signal raced the child's exit. Decode waitid results in one failable initializer, TerminationStatus(ifTerminated:), shared by _peekIfExited and _reap. _blockingReap now retries waitid after a CLD_STOPPED result until the child terminates; other unexpected codes still trap. Add regression tests for reap, blocking reap and teardown of a stopped child, and check that monitoring waits for a stopped child's resume.
81082b2 to
acce7fe
Compare
Summary
This change makes
_peekIfExitedrecognize only terminal child states:CLD_EXITED,CLD_KILLED, andCLD_DUMPED.It addresses unexpected
waitidbehavior reproduced on macOS 27 beta and macOS 26.6.2: an exit-only query returned a stop notification for a child that was still alive and suspended.This is proposed as defensive handling of the observed behavior, rather than a claim that the existing condition is incorrect on every platform.
Expected and observed behavior
For a child suspended with
SIGSTOP, this call should not report a terminal exit:With a freshly zero-initialized
siginfo_t, the observed result was:Because
si_pidandsi_signoare nonzero, the original_peekIfExitedcondition returnstrue, although the child is paused rather than terminated. This can bypass exit monitoring and lead to a nonterminal status reachingTerminationStatus, which traps on unexpected status codes.The proposed condition returns
falsefor this stop notification.Reproduction and validation
macOS 27.0 beta — build
26A5425aEnvironment:
arm64)27A5252fswiftlang-6.4.0.33.1)The PR includes two Swift regression tests:
testStoppedChildIsNotReportedAsExited: suspend a child and verify that exit observation returns false without consuming the pending stop notification.testMonitoringStoppedChildWaitsForItsFinalExit: monitor a stopped child, resume it, and verify its final exit status is 42.Both tests passed with the patch in the latest local run on this environment.
An independent C diagnostic reproduced the unexpected OS result in 80/80 trials: 20 trials for each combination of
fork/posix_spawnand exit queries with/withoutWNOWAIT.That diagnostic observed the paused state through
proc_pidinfo, without any precedingwaitidcall. All 80 children subsequently resumed and exited with status 42. A live, non-paused child returned zero PID, signal, and code as a negative control.Summary lines from that run:
Here, “old false exits” counts how often the original nonzero-PID/signal condition would classify a paused child as exited. “Subsequent real exits” counts the later
CLD_EXITEDresults with status 42, after explicitly resuming the children.macOS 26.6.2 — build
25G83A smaller, single-case C diagnostic also reproduced the behavior using
forkandWEXITED | WNOHANG | WNOWAIT.Recorded output:
nonzero PID=1is a Boolean indicator, not the child's actual PID. The other values correspond toSIGCHLD = 20,CLD_STOPPED = 5, andSIGSTOP = 17.This extends the reproduction evidence beyond macOS 27 beta. The 80-trial matrix, the query without
WNOWAIT, and the Swift regression tests have not been run on macOS 26.6.2 as part of this validation.Standalone reproduction
The source below is the smaller diagnostic used for the macOS 26.6.2 result, not the earlier 80-trial matrix.
It does not link against Swift or Swift Subprocess. It creates its own child, confirms the paused state through
proc_pidinfo, and then makes the exit-onlywaitidquery with freshly initialized output. It subsequently resumes the child and checks its actual exit status.It includes cleanup for normal exits and interruption/timeout signals.
waitid-check.c.C diagnostic source
Interpretation and scope
The diagnostics demonstrate that an exit-only query can return
CLD_STOPPEDon the two tested macOS versions, and that the original library condition misclassifies that result as terminal.Apple's published XNU implementation gates stop notifications on
WSTOPPED. The observed behavior therefore warrants investigation at the OS boundary as well as consideration of defensive handling in the library.The C diagnostics isolate the OS behavior; they do not themselves execute the Swift library's subsequent crash path.
The investigation began with application subprocess hangs. After adopting the patched library, I observed the application working normally again. However, I haven't established that this specific condition caused the original hangs, so I’m treating that improvement as supporting context rather than proof of the root cause.
Remaining review
_reap, if that approach is agreed upon.The adjacent
_reaphelper still uses the original nonempty-result check. The macOS 27 diagnostic reproduced the same stop notification withoutWNOWAIT, so that path also needs consideration and focused regression coverage. The current patch does not address it.Feedback is welcome on whether this approach is appropriate or whether a different solution would be preferable.