Skip to content

Commit 2cee899

Browse files
committed
Process: correct a subtle runLoopSource lifetime issue
When a process is run with `run`, a monitoring thread is setup by means of a socketpair. This socket is used to create a runloop source, which will monitor the process for termination. `waitUntilExit` will monitor the `isRunning` ivar which is set by the source to indicate the process termination having been observed. We however would previously mark the process as terminated before invalidating the runloop source to prevent any new callouts on the source. However, the operation is multithreaded the operation is not guaranteed to be serialized and the runloop source may be `nil`'ed prior or during the invalidation which also releases as the refcount is now 0. This would result in the `runloopSource` passed to `CFRunLoopSourceInvalidate` being `nil` and breaking the contract of the call or a UaF if the value is `nil`'ed during the execution. These states have both been observed in practice. Reordering the `isRunning` ivar assignment to occur after the invalidation ensures that the value remains usable until we no longer reference it.
1 parent efedfc3 commit 2cee899

File tree

1 file changed

+11
-7
lines changed

1 file changed

+11
-7
lines changed

Sources/Foundation/Process.swift

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -642,17 +642,21 @@ open class Process: NSObject {
642642
process._terminationReason = .exit
643643
}
644644

645-
if let handler = process.terminationHandler {
646-
let thread: Thread = Thread { handler(process) }
647-
thread.start()
645+
// Invalidate the source and wake up the run loop, if it is available.
646+
CFRunLoopSourceInvalidate(process.runLoopSource)
647+
if let runloop = process.runLoop {
648+
CFRunLoopWakeUp(runloop._cfRunLoop)
648649
}
649650

651+
// Ensure that the runLoop is invalidated before we mark the process
652+
// as no longer running. This serves as a semaphore to
653+
// `waitUntilExit` to decrement the `runLoopSource` retain count,
654+
// potentially releasing it.
650655
process.isRunning = false
651656

652-
// Invalidate the source and wake up the run loop, if it is available
653-
CFRunLoopSourceInvalidate(process.runLoopSource)
654-
if let runloop = process.runLoop {
655-
CFRunLoopWakeUp(runloop._cfRunLoop)
657+
if let handler = process.terminationHandler {
658+
let thread: Thread = Thread { handler(process) }
659+
thread.start()
656660
}
657661

658662
CFSocketInvalidate(socket)

0 commit comments

Comments
 (0)