Skip to content

fix: Rare race in cluster tests - #3194

Merged
godexsoft merged 1 commit into
XRPLF:developfrom
godexsoft:bugfix/race-in-node-test
Aug 28, 2026
Merged

fix: Rare race in cluster tests#3194
godexsoft merged 1 commit into
XRPLF:developfrom
godexsoft:bugfix/race-in-node-test

Conversation

@godexsoft

Copy link
Copy Markdown
Collaborator

While working on #3174 we had a failure in https://github.com/XRPLF/clio/actions/runs/33074493704/job/98527800974?pr=3174 which appears to be an actual race. This PR fixes the small and rare race that the run exposed.

Backend::run() starts two independent RepeatedTasks on separate strands: a reader
(doReadonNewState_) and a writer (doWritewriteNodeMessage). Seven tests
asserted EXPECT_CALL(writeNodeMessage).Times(AtLeast(1)) but only ever waited on the
read path (semaphore.acquire() is released from the state callback). When the test body
returns, ~Backendstop() sets state_ = Stopped, and RepeatedTask's loop checks
state_ after the timer wait — so a writer whose first tick hasn't landed yet breaks
out without ever calling doWrite(). CI hit exactly that: writeNodeMessage
"never called - unsatisfied and active".

Both intervals are 1 ms, so the writer almost always wins the race — hence the rarity.

  • Relaxed those seven expectations to Times(AnyNumber()). None of them inspects the
    written message; the expectation only existed because the fixture is a StrictMock.
    The write path stays properly covered by the two WriteNodeMessage* tests, which
    release the semaphore from inside the write action and so actually wait for it.
    Stop keeps AtLeast(1) (it sleeps 20 ms and asserting both tasks ran is its point).
  • std::binary_semaphorestd::counting_semaphore<> in the fixture: both callbacks
    are WillRepeatedly on a 1 ms timer, so a second release() before the main thread's
    acquire() was UB.

@godexsoft
godexsoft requested a review from kuznetsss August 27, 2026 15:03
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@godexsoft
godexsoft merged commit e5f11df into XRPLF:develop Aug 28, 2026
31 checks 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.

2 participants