Skip to content

Commit 27acc07

Browse files
pcbeardcompnerd
andcommitted
Refactor CFSocket callbacks to call Process.terminateRunLoop()
This fixes a data race between setting `process.isRunning = false` and then calling `CFRunLoopSourceInvalidate(process.runLoopSource)`. Process.waitUntilExit() may wake up and clear .runLoopSource which results in a fairly common crash on Windows. Put this logic in a common method to reduce drift between the Darwin / Linux and Windows code paths. Co-authored-by: Saleem Abdulrasool <[email protected]>
1 parent 2cee899 commit 27acc07

File tree

1 file changed

+25
-35
lines changed

1 file changed

+25
-35
lines changed

Sources/Foundation/Process.swift

Lines changed: 25 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -642,22 +642,8 @@ open class Process: NSObject {
642642
process._terminationReason = .exit
643643
}
644644

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)
649-
}
650-
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.
655-
process.isRunning = false
656-
657-
if let handler = process.terminationHandler {
658-
let thread: Thread = Thread { handler(process) }
659-
thread.start()
660-
}
645+
// Signal waitUntilExit() and optionally invoke termination handler.
646+
process.terminateRunLoop()
661647

662648
CFSocketInvalidate(socket)
663649
}, &context)
@@ -833,28 +819,12 @@ open class Process: NSObject {
833819
process._terminationReason = .exit
834820
}
835821

836-
// Set the running flag to false
837-
process.isRunning = false
838-
839-
// If a termination handler has been set, invoke it on a background thread
840-
841-
if let terminationHandler = process.terminationHandler {
842-
let thread = Thread {
843-
terminationHandler(process)
844-
}
845-
thread.start()
846-
}
847-
848-
// Invalidate the source and wake up the run loop, if it's available
849-
850-
CFRunLoopSourceInvalidate(process.runLoopSource)
851-
if let runLoop = process.runLoop {
852-
CFRunLoopWakeUp(runLoop._cfRunLoop)
853-
}
822+
// Signal waitUntilExit() and optionally invoke termination handler.
823+
process.terminateRunLoop()
854824

855825
CFSocketInvalidate( socket )
856826

857-
}, &context )
827+
}, &context )
858828

859829
CFSocketSetSocketFlags( socket, CFOptionFlags(kCFSocketCloseOnInvalidate))
860830

@@ -1169,6 +1139,26 @@ open class Process: NSObject {
11691139
self.runLoop = nil
11701140
self.runLoopSource = nil
11711141
}
1142+
1143+
private func terminateRunLoop() {
1144+
// Ensure that the run loop source is invalidated before we mark the process
1145+
// as no longer running. This serves as a semaphore to
1146+
// `waitUntilExit` to decrement the `runLoopSource` retain count,
1147+
// potentially releasing it.
1148+
CFRunLoopSourceInvalidate(self.runLoopSource)
1149+
let runloopToWakeup = self.runLoop
1150+
self.isRunning = false
1151+
1152+
// Wake up the run loop, *AFTER* clearing .isRunning to avoid an extra time out period.
1153+
if let cfRunLoop = runloopToWakeup?._cfRunLoop {
1154+
CFRunLoopWakeUp(cfRunLoop)
1155+
}
1156+
1157+
if let handler = self.terminationHandler {
1158+
let thread: Thread = Thread { handler(self) }
1159+
thread.start()
1160+
}
1161+
}
11721162
}
11731163

11741164
extension Process {

0 commit comments

Comments
 (0)