Skip to content

Unified implementation for withTimeout family#63

Open
rintaro wants to merge 2 commits into
swiftlang:mainfrom
rintaro:withtimout-continuation
Open

Unified implementation for withTimeout family#63
rintaro wants to merge 2 commits into
swiftlang:mainfrom
rintaro:withtimout-continuation

Conversation

@rintaro
Copy link
Copy Markdown
Member

@rintaro rintaro commented May 29, 2026

Introduce withTimeoutResult(_:body:resultReceivedAfterTimeout:) returning a WithTimeoutResult<T> enum (.result(Result<T>)/ .timedOut). Existing withTimeout overloads become thin switches over it. The optional late-result callback receives successful late results only.

withTaskPriorityChangedHandler now delegates to stdlib's withTaskPriorityEscalationHandler on macOS 26+, giving event-driven priority reaction instead of polling.

Also withTaskPriorityChangedHandler and all the withTimeout overloads are now rethrows.

Added some test cases to ensure old withTaskPriorityChangedHandler implementation keep working even when testing with withTaskPriorityEscalationHandler.

On platforms with SwiftStdlib 6.2 (macOS/iOS/macCatalyst 26+),
delegate to withTaskPriorityEscalationHandler. The new API is
event-driven (no polling) and integrated with the runtime, so it
reacts immediately to escalations.

Mark the wrapper @available(..., deprecated: 26.0). Once the min
deployment target reaches 26, the deprecation warning fires at
every call site, prompting migration to
withTaskPriorityEscalationHandler and removal of this wrapper.

Change the function from `throws` to `rethrows` following the stdlib
version. Also, use `RefBox<Atomic<_>>` for the previous priority
storage. Atomic is lighter, and actually simplifies the code.

The old codepath is extracted as a function and kept tested even on
platforms with built-in withTaskPriorityEscalationHandler.
if didHitTimeout.value {
await resultReceivedAfterTimeout(result)
}
continuation.yield(result)
Copy link
Copy Markdown
Member Author

@rintaro rintaro May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was actually a correctness issue here.
After didHitTimeout.value check passes but before continuation.yield(result), the timeout task might have set didHitTimeout and continuation.yield(nil). So the result is timeout but resultReceivedAfterTimeout never get called.

@rintaro rintaro marked this pull request as draft May 29, 2026 21:33
@rintaro rintaro force-pushed the withtimout-continuation branch 2 times, most recently from a049383 to 4881218 Compare May 29, 2026 23:00
Introduce `withTimeoutResult(_:body:resultReceivedAfterTimeout:)` as a
unified core of the `withTimeout` family. The existing `withTimeout`
overloads become thin `@inlinable` switches over `WithTimeoutResult`,
forwarding errors via `rethrows`.

`withTimeoutResult` supports an optional late-result callback. Late
errors after the timeout are dropped silently because the callback
signature accepts only the success value, so errors have nowhere to go.

Implementation note: `bodyTask` itself yields its outcome onto the
internal `AsyncStream` instead of a separate forwarder task. Adding
a forwarder that awaits `bodyTask.result` proved unreliable — priority
escalation through the await chain didn't always reach `bodyTask`,
leaving `Task.currentPriority` inside `body` stuck at the original
priority.
@rintaro rintaro force-pushed the withtimout-continuation branch from 4881218 to f8a0aa3 Compare May 29, 2026 23:20
@rintaro rintaro marked this pull request as ready for review May 29, 2026 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant