test: close statically proven CI race causes - #649
Merged
Merged
Conversation
SunSi12138
marked this pull request as ready for review
September 10, 2026 10:34
SunSi12138
commented
Sep 10, 2026
SunSi12138
left a comment
Owner
Author
There was a problem hiding this comment.
整体两个 root-cause 方向都合理,第二个 phase-gate flush scheduler-bound 修复也有 #387 的 exact-head recurrence 证据。当前唯一 blocker 是 tracked-unary 新 hook 仍可能被 ConnectAsync 后立即启动的首个 heartbeat Ping 抢走,因此目标 Request 的 emission ordering edge 还没有完全建立,见 inline。
SunSi12138
marked this pull request as draft
September 10, 2026 10:35
SunSi12138
marked this pull request as ready for review
September 10, 2026 12:36
SunSi12138
commented
Sep 10, 2026
SunSi12138
left a comment
Owner
Author
There was a problem hiding this comment.
上次唯一 blocker 已修复:tracked-unary 测试现在先通过 ready session 的 FlushSendQueueAsync 排空 ConnectAsync/首个 heartbeat Ping 的既有输出,再 arm one-shot output-buffer hook,因此 callback 才真正唯一绑定到目标 Unary 的 emission boundary。第二处 phase-gate flush 改动仍只移除语义 barrier 外层的 scheduler-bound timeout。exact-head Fast / Extended / CodeQL 均通过,未发现新的工程级问题。
30 tasks
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
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.
Scope
This PR groups only root causes that can be closed from the current source/CI evidence. Each root cause is kept in a separate commit; no timeout widening, blanket retries, quarantine, or production behavior changes are used as a substitute for causality.
1. Tracked unary deadline at emission
SharpLinkClientTrackedEmissionDeadlineTests.TimedUnaryDroppedAtEmissionShouldCompleteWithoutDeadlineTimerCallbackadvanced manual time immediately after startingInvokeUnaryAsync, then used a later send-queue flush as if it proved the target Request had reached emission. There is no ordering edge between invocation scheduling and that flush marker, so the marker can complete first; because the test intentionally does not run deadline timers, the invocation can then remain pending until the test watchdog.The test now uses the existing
TestTransportConnection.RunOnNextOutputBufferRequestseam. Before arming the one-shot hook, it drains output already associated withConnectAsyncthrough the ready session's semanticFlushSendQueueAsyncbarrier, matching the established #392 pattern and excluding the initial heartbeat Ping from consuming the hook. Manual time therefore advances only when the send pump has actually reached the target Unary Request's output-buffer acquisition, immediately before the production emission path re-samples the deadline. This preserves the assertion that the expired Request never reaches transport without adding a wall-clock timeout or retry.2. Protocol phase gate flush scheduler bound
NegotiatedSessionOptionsTests.SendPumpShouldEnforceProtocolPhaseBeforeQueueinghad already completed the phase-rejection checks and queued the allowed drainingResponse, then wrapped the semanticFlushSendQueueAsyncbarrier in an unrelated two-second wall-clockWaitAsync.The flush itself is retained because the final queue-accounting assertion requires the allowed cleanup frame to drain. Only the local two-second scheduler clock is removed.
Static-audit rule
The remaining #387 records were rechecked against current
dev; a production-path fix is included only when state/ownership/protocol behavior can be proven wrong from the source. Historical items already closed by merged PRs are not duplicated, and unresolved one-off evidence is not changed speculatively.Review follow-up
2be9488d: establish a deterministic pre-existing-output barrier before the tracked-unary emission hook, so the first heartbeat Ping cannot consume the callback.Validation
CI is running on exact head
2be9488d386fa31d01ee3306dabd77e13205e78c.