Skip to content

fix: close deadline arm race and stabilize #387 lifecycle regressions - #698

Merged
SunSi12138 merged 10 commits into
devfrom
fix/ci-flakes-387-20260917
Sep 18, 2026
Merged

SunSi12138 merged 10 commits into
devfrom
fix/ci-flakes-387-20260917

Conversation

@SunSi12138

@SunSi12138 SunSi12138 commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner

Refs #387.

Root causes

  • 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 ServerStreamDisposed WaitAsync 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.

Copy link
Copy Markdown
Owner Author

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)
  • allocation gate: success
  • CodeQL #4040: success
  • PR Package Smoke release: stabilize 1.1.1 GoAway gate #58: success.

@SunSi12138
SunSi12138 merged commit e355e0f into dev Sep 18, 2026
6 of 9 checks passed
@SunSi12138
SunSi12138 deleted the fix/ci-flakes-387-20260917 branch September 18, 2026 10:00
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