You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: close deadline arm race and stabilize #387 lifecycle regressions - #698
macOS / RetryShouldHonorDeadlineAndCancellationDuringDelay: this exposed a production TOCTOU in SharpLinkTimer.WaitAsync(Task, RpcDeadline, ...). The helper sampled the absolute deadline's remaining duration and then armed a relative Task.WaitAsync timer. If the clock crossed the deadline between those two operations, the timer was armed from the later clock value with the stale remaining duration. With ManualTimeProvider, one Advance could therefore leave the waiter parked until another advance; on a real provider the same race permits deadline overshoot.
Ubuntu / DisconnectedCleanupShouldReleaseTopologyGateAndRemainSupervised(static): the production ownership path is balanced. PendingRequestTable.CompleteTakenCall publishes the RPC operation result before IPendingCallOwner.OnPendingCallCompleted and ReleaseSlot, while disconnected cleanup is explicitly detached and framework-supervised. The test joined the RPC task and immediately asserted physical ownership, so it could observe the legal tail of the supervised cleanup. The static pool widened that scheduling window.
Dynamic ABI soak / consumer-break: consumer disposal publishes local abandonment and sends protocol Cancel, but it is not an acknowledgement that the peer has already consumed that cancel and executed the server iterator finally. The 2-second ServerStreamDisposedWaitAsync was therefore a wall-clock latency budget on asynchronous remote cleanup, not a semantic barrier; repeated ABI soak eventually exceeded it.
Changes
For task/deadline waits, create the deadline timer disarmed first so timer ownership exists before projecting the absolute deadline into a relative due time. Only then re-sample the absolute remaining budget and program the owned timer. This prevents time consumed inside CreateTimer from shifting the effective deadline.
Keep deadline/source/caller-cancellation arbitration explicit. An already-completed source retains its existing fast-path priority; otherwise a caller cancellation that is already terminal is observed before timer ownership begins, so CreateTimer cannot advance the provider to the deadline and overwrite that earlier cancellation. If the deadline signal later wins, cancellation of the losing source waiter cannot replace the already-expired deadline outcome.
Add deterministic regressions for the timer-arm races: crossing the whole 5-second deadline inside CreateTimer; advancing only 2 seconds during timer creation then advancing the remaining 3 seconds to the exact absolute deadline; and pre-canceling the caller token before a CreateTimer that advances the clock to the deadline, verifying both cancellation outcome and original token identity.
In the disconnected-cleanup regression, keep the existing assertion that cleanup is supervised, then wait for the DisconnectedConnectionCleanup operation to leave the supervisor's active snapshot before asserting reservation/active-call ownership is zero.
Remove the per-round 2-second ServerStreamDisposed guard. The test still waits for the actual server iterator disposal signal, while the enclosing test timeout remains the guard for a genuinely lost cancellation or stuck iterator.
Validation
Static analysis and GitHub-side diff review performed here. This execution environment has no local .NET SDK; executable validation is provided by PR CI.
Branch is based on dev at 708b9d5f286e2047891cd960c6ea8e85cda1498a.
Blocker addressed on head c9c25891cc97072bf87a39a2fcc29e47c46abc50.
The previous post-arm IsExpired check was insufficient for partial arm drift. The task/deadline wait now establishes an ITimer in the disarmed state first, then re-samples the absolute RPC deadline and programs that already-owned timer from the fresh remaining budget. This removes the stale pre-ownership relative timeout rather than only detecting the case where timer creation crossed the whole deadline.
Added the deterministic regression described in review:
absolute deadline = 5s
provider advances 2s inside the first CreateTimer
advance another 3s to exactly t=5s
wait must return false
active timer count must return to zero
The original full-crossing regression remains as well. During follow-up arbitration review I also fixed the tied caller-cancellation path so cancellation of the losing source waiter cannot overwrite a deadline signal that already won.
Validation on this exact head:
PR Fast #1680: success (format, maintainability, Release build, Unit, Generator, Load Test Tests)
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
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.
Refs #387.
Root causes
RetryShouldHonorDeadlineAndCancellationDuringDelay: this exposed a production TOCTOU inSharpLinkTimer.WaitAsync(Task, RpcDeadline, ...). The helper sampled the absolute deadline's remaining duration and then armed a relativeTask.WaitAsynctimer. If the clock crossed the deadline between those two operations, the timer was armed from the later clock value with the stale remaining duration. WithManualTimeProvider, oneAdvancecould therefore leave the waiter parked until another advance; on a real provider the same race permits deadline overshoot.DisconnectedCleanupShouldReleaseTopologyGateAndRemainSupervised(static): the production ownership path is balanced.PendingRequestTable.CompleteTakenCallpublishes the RPC operation result beforeIPendingCallOwner.OnPendingCallCompletedandReleaseSlot, while disconnected cleanup is explicitly detached and framework-supervised. The test joined the RPC task and immediately asserted physical ownership, so it could observe the legal tail of the supervised cleanup. The static pool widened that scheduling window.consumer-break: consumer disposal publishes local abandonment and sends protocolCancel, but it is not an acknowledgement that the peer has already consumed that cancel and executed the server iteratorfinally. The 2-secondServerStreamDisposedWaitAsyncwas therefore a wall-clock latency budget on asynchronous remote cleanup, not a semantic barrier; repeated ABI soak eventually exceeded it.Changes
CreateTimerfrom shifting the effective deadline.CreateTimercannot advance the provider to the deadline and overwrite that earlier cancellation. If the deadline signal later wins, cancellation of the losing source waiter cannot replace the already-expired deadline outcome.CreateTimer; advancing only 2 seconds during timer creation then advancing the remaining 3 seconds to the exact absolute deadline; and pre-canceling the caller token before aCreateTimerthat advances the clock to the deadline, verifying both cancellation outcome and original token identity.DisconnectedConnectionCleanupoperation to leave the supervisor's active snapshot before asserting reservation/active-call ownership is zero.ServerStreamDisposedguard. The test still waits for the actual server iterator disposal signal, while the enclosing test timeout remains the guard for a genuinely lost cancellation or stuck iterator.Validation
Static analysis and GitHub-side diff review performed here. This execution environment has no local .NET SDK; executable validation is provided by PR CI.
Branch is based on
devat708b9d5f286e2047891cd960c6ea8e85cda1498a.