Skip to content

Fix race in ConnectionsCountForAPollingSubscriptionChangesOneConnectionAtATime - #735

Merged
gb-8 merged 1 commit into
mainfrom
lev-2276/fix-halibut-polling-connection-observer-test
Nov 9, 2026
Merged

gb-8 merged 1 commit into
mainfrom
lev-2276/fix-halibut-polling-connection-observer-test

Conversation

@gb-8

@gb-8 gb-8 commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Background

ConnectionsCountForAPollingSubscriptionChangesOneConnectionAtATime is flaky due to a race condition.
This PR fixes it by adding a retry loop waiting for the eventual state to be reached.

Details

After the port forwarder drops the polling connection, the next request fails on the server's dead connection. In MessageExchangeProtocol.ProcessReceiverInternalAsync, the server:

  1. calls pendingRequests.ApplyResponse(...) with the error, which completes the client's SayHelloAsync, and then
  2. returns false, which exits the using (LeaseActiveTcpConnection(...)) block in ExchangeAsServerAsync and only then reports the 1 → 0 count change.

The test resumes after step 1 and immediately reads the second observed event, which may not have been recorded yet.

Results

Before

ConnectionsCountForAPollingSubscriptionChangesOneConnectionAtATime intermittently failed with InvalidOperationException: Sequence contains no elements (1 in ~97 runs on Windows 2019).

After

We loop in the test and wait for the expected connections count change.

Testing

ConnectionObserverFixture passes locally on net10.0 (9/9). The original failure is rare, so a local pass doesn't prove the race is gone.

…rFixture

The server completes the failed request before it releases the connection
lease, so the 1 -> 0 count change can be recorded after the client's
SayHelloAsync has already returned. Retry the assertion as the final close
check already does.

Fixes LEV-2276

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@gb-8
gb-8 merged commit 4d7a637 into main Nov 9, 2026
14 checks passed
@gb-8
gb-8 deleted the lev-2276/fix-halibut-polling-connection-observer-test branch November 9, 2026 00:56
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.

2 participants