Skip to content

Fix race in TenantBatchQueue composite-member test (holder not yet dequeued) - #1144

Merged
jeremydmiller merged 1 commit into
mainfrom
fix-tenant-batch-queue-composite-test-race
Nov 8, 2026
Merged

jeremydmiller merged 1 commit into
mainfrom
fix-tenant-batch-queue-composite-test-race

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Hardens the intermittently failing EventTests.Daemon.TenantBatchQueueTests.a_composite_member_runs_inside_its_parents_turn_without_queueing. It failed on main CI (net9.0), on generator-runtime-version-1138 (net9.0 + net10.0), and on #1142 (net10.0), always at queue.Waiting.ShouldBe(0) reading 1.

Cause: a race in the test, not a product bug

TenantBatchQueue.RunAsync increments _waiting and posts to the block. _waiting is only decremented when the block's single reader dequeues the item. That reader is a Task.Run worker, resumed asynchronously because the channel has AllowSynchronousContinuations = false.

The test posts a "holder" batch to occupy the queue, then runs the composite member right away. With NSubstitute fakes the member's ProcessRangeAsync finishes almost synchronously. Under load (CI runners, the whole suite in parallel), the reader often hasn't been scheduled yet, so the holder is still counted. Waiting reads 1 from the holder, not from the member.

I confirmed this with a temporary diagnostic that recorded Waiting before and after the member ran. All 14 failures it caught showed before=1 after=1, and the count drained to 0 on its own once the reader ran. The composite path in ProjectionExecution.processRangeAsync never touches the queue: it only queues when range.BatchBehavior == BatchBehavior.Individual.

Fix

The holder now signals when it has actually taken the queue's turn. The test waits for that signal, and checks Waiting == 0, before running the member. The original assertions are unchanged, so they only measure what the composite member did. No timeouts were widened and no retries were added.

The test still catches the regression it exists for. As a mutation check, I temporarily made ProcessRangeAsync queue composite members too. The hardened test failed with a TimeoutException, because the member deadlocks behind the holder. That change was reverted.

Evidence

I ran the full EventTests suite (1831 tests) in a loop, using the built test executable, with 36 yes > /dev/null CPU hogs on an 18-core machine. Running this test alone never reproduced the failure (30/30 passes under the same hogs), because it needs the rest of the suite competing for the thread pool.

net10.0 net9.0
Before (main) 25 / 50 runs failed this test 31 / 50 runs failed this test
After 0 / 60 failed (60/60 clean suite runs) 0 / 60 failed (60/60 clean suite runs)

No other test failed in any of those runs.

🤖 Generated with Claude Code

….Waiting

a_composite_member_runs_inside_its_parents_turn_without_queueing read
queue.Waiting right after the member ran, but the holder batch posted at
the top of the test counts as Waiting until the queue's single reader
(a thread-pool task) dequeues it. Under load the reader had not run yet,
so Waiting read 1 from the holder, not from the member.

The holder now signals when it holds the turn and the test waits for it,
so the assertion only sees what the composite member did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jeremydmiller
jeremydmiller merged commit db11f94 into main Nov 8, 2026
1 check passed
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