Skip to content

Commit 34925be

Browse files
committed
Fix a race condition bug involving timers and async tasks
1 parent ba779e9 commit 34925be

File tree

2 files changed

+33
-0
lines changed

2 files changed

+33
-0
lines changed

ios/PacketTunnelCore/Actor/Task+Duration.swift

+11
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,21 @@ extension Task where Success == Never, Failure == Never {
4040

4141
try await withTaskCancellationHandler {
4242
try await withCheckedThrowingContinuation { continuation in
43+
// The `continuation` handler should never be `resume`'d more than once.
44+
// Setting the eventHandler on the timer after it has been cancelled will be ignored.
45+
// https://github.com/apple/swift-corelibs-libdispatch/blob/77766345740dfe3075f2f60bead854b29b0cfa24/src/source.c#L338
46+
// Therefore, set a flag indicating `resume` has already been called to avoid `resume`ing more than once.
47+
// Cancelling the timer does not cancel an event handler that is already running however,
48+
// the cancel handler will be scheduled after the event handler has finished.
49+
// https://developer.apple.com/documentation/dispatch/1385604-dispatch_source_cancel
50+
// Therefore, there it is safe to do this here.
51+
var hasResumed = false
4352
timer.setEventHandler {
53+
hasResumed = true
4454
continuation.resume()
4555
}
4656
timer.setCancelHandler {
57+
guard hasResumed == false else { return }
4758
continuation.resume(throwing: TaskCancellationError())
4859
}
4960
timer.schedule(wallDeadline: .now() + DispatchTimeInterval.milliseconds(duration.milliseconds))

ios/PacketTunnelCoreTests/TaskSleepTests.swift

+22
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,26 @@ final class TaskSleepTests: XCTestCase {
2525
XCTAssert(error is CancellationError)
2626
}
2727
}
28+
29+
/// This test triggers a race condition in `sleepUsingContinuousClock` where an `AutoCancellingTask` will
30+
/// cancel a `DispatchSourceTimer` in a `Task` trying to call `resume` on its continuation handler more than once
31+
func testSuccessfulEventHandlerRemovesCancellation() async throws {
32+
for _ in 0 ... 20 {
33+
var task = recoveryTask()
34+
try await Task.sleep(duration: .milliseconds(10))
35+
task.doAnythingToSilenceAWarning()
36+
}
37+
}
38+
39+
private func recoveryTask() -> AutoCancellingTask {
40+
AutoCancellingTask(Task.detached {
41+
while Task.isCancelled == false {
42+
try await Task.sleepUsingContinuousClock(for: .milliseconds(10))
43+
}
44+
})
45+
}
46+
}
47+
48+
private extension AutoCancellingTask {
49+
func doAnythingToSilenceAWarning() {}
2850
}

0 commit comments

Comments
 (0)