test: fix race in connect_tests disconnect-deferred-failure test - #343
test: fix race in connect_tests disconnect-deferred-failure test#343ryanofsky wants to merge 1 commit into
Conversation
The "ConnectStream defers disconnect failure" test was flaky: there is a race between the event loop detecting the disconnect and foo->add() being called. If the onDisconnect callback fires first and nulls m_context.connection, the error is "called after disconnect"; if foo->add() submits before the callback fires, the error is "interrupted by disconnect". Accept both, with a comment explaining the race. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste |
|
ACK 137a6e4 I couldn't reproduce the "IPC client method call interrupted by disconnect." path locally, so I'm curious what circumstances trigger it. In any case the changes look good to me, nice comment explaining the race. |
I think it might help to have a slower machine, since it happens when the event loop thread is slow to process the disconnect. Some failures from bitcoin/bitcoin#32387 were https://github.com/bitcoin/bitcoin/actions/runs/31655880824/job/94310097312 |
That makes sense, thanks! |
3 participants
This fixes an intermittent
mptestfailure which is a regression from #308. Commit bb47369 introduced a test with a race condition where there the test could trigger two different exceptions depending on when the client onDisconnect handler ran, and only one of the exceptions was being checked for in the test. If the onDisconnect handler ran later, the test would fail.