Add a withDeadline structured-concurrency helper - #432
Draft
eseay wants to merge 2 commits into
Draft
Conversation
eseay
force-pushed
the
eddie/with-deadline-helper
branch
from
July 31, 2026 15:57
d2c7114 to
376a47d
Compare
rebello95
reviewed
Aug 4, 2026
rebello95
left a comment
rebello95
left a comment
Collaborator
There was a problem hiding this comment.
Would be great to see a (stacked?) PR demonstrating how this will be used before merging dead code
| /// | ||
| /// - returns: The result of `operation`, or `nil` if the deadline elapsed first. | ||
| func withDeadline<T: Sendable>( | ||
| _ timeout: TimeInterval?, |
Collaborator
There was a problem hiding this comment.
This should probably be non-optional, since this function provides no additive value in the nil case
Contributor
Author
There was a problem hiding this comment.
Yea we can leave this open until the other stuff is done. I like that idea.
eseay
marked this pull request as draft
August 8, 2026 16:36
Adds the primitive that will replace TimeoutTimer's four-state machine as the async migration proceeds. withDeadline races an operation against a deadline in a task group and returns nil if the deadline wins, canceling the operation. TimeoutTimer exists to reconcile a timer against callback ordering, hence its sticky-cancel invariant and deinit-disarm. Under structured concurrency the race is the mechanism, and losing it is directly observable, so the correlation currently duplicated at ProtocolClient.swift:123 and :344 becomes unnecessary. Pure addition: nothing calls this yet and no existing file is modified. TimeoutTimer and its tests are untouched and will be removed once ProtocolClient is inverted. Covered by 6 tests, including a regression test asserting a won race returns promptly -- without group.cancelAll() the helper would block for the full deadline, which is otherwise invisible. Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
eseay
force-pushed
the
eddie/with-deadline-helper
branch
from
August 8, 2026 18:11
376a47d to
5abc5d9
Compare
Addresses review feedback: the helper provided no additive value in the nil case, where it just forwarded straight to operation(). Now that the first caller exists, the nil branch is the caller's concern -- ProtocolClient's config.timeout is optional, so it unwraps and picks between a deadlined and an undeadlined call rather than pushing that choice down here. Drops the guard-let branch and the test covering it. Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
withDeadline, a small structured-concurrency helper that races an operation against a deadline and cancels the loser. It's unused for now — a pure addition, laying groundwork for replacingTimeoutTimer's callback-based state machine in a later change.withDeadline(_:operation:)inDeadline.swiftracesoperationagainstTask.sleepin aTaskGroup, returningnilif the deadline wins.TimeoutTimerand its call sites are untouched.Follow-up
TimeoutTimer.swiftis a four-state machine that exists to reconcile a timer against callback-based ordering — sticky cancellation, a deinit-disarm, and a.canceled && timedOut → .deadlineExceededcorrelation that's currently duplicated in two places inProtocolClient.swift. None of that reconciliation is needed once callers can justawait withDeadline(...): losing the race is directly observable, so there's nothing left to correlate.The plan is for
ProtocolClient's unary and streaming request paths to adoptwithDeadlinedirectly in a follow-up change. Once nothing references it,TimeoutTimer.swiftand its dedicated unit tests get deleted outright. The existing end-to-end timeout test that exercises real behavior throughProtocolClientstays and gets adjusted in place, since it's pinning behavior rather than the state machine.Keeping this PR scoped to just the helper — with no consumers yet — keeps it small and independently reviewable.
Test plan
swift build && swift test— 92/92 passing, suite completes in ~3sDeadlineTestscovers: operation wins, deadline wins, no deadline, negative-timeout clamping, operation cancellation on a lost race, and a regression check that a won race returns promptly rather than blocking for the full deadline