Fix race condition in vote_router::disconnect (...)#5010
Fix race condition in vote_router::disconnect (...)#5010pwojcikdev merged 1 commit intonanocurrency:developfrom
vote_router::disconnect (...)#5010Conversation
a72d565 to
73c793d
Compare
Test Results for Commit 7ab66ecPull Request 5010: Results Test Case Results
Last updated: 2026-01-25 13:58:10 UTC |
There was a problem hiding this comment.
Pull request overview
Fixes an intermittent assertion failure caused by a race between election::replace_by_weight() and active_elections::erase_election() by making vote_router::disconnect(...) tolerant of already-erased routes.
Changes:
- Change
vote_router::disconnect(block_hash)to returnboolinstead of asserting the route existed. - Change
vote_router::disconnect(election)to returnstd::size_twith number of routes removed. - Minor header/cpp reorganization and comment/doc updates for
vote_router.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| nano/node/vote_router.hpp | Updates API signatures/docs; switches condition variable type to nano::condition_variable. |
| nano/node/vote_router.cpp | Implements new return values for disconnect methods and reorders start/stop definitions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mutable std::shared_mutex mutex; | ||
| nano::condition_variable condition; | ||
| std::thread thread; |
There was a problem hiding this comment.
condition was switched to nano::condition_variable while mutex remains a std::shared_mutex. When NANO_TIMED_LOCKS>0, nano::condition_variable only supports waiting on nano::unique_lock<nano::mutex>, so condition.wait_for(std::unique_lock<std::shared_mutex>&, ...) in run() will not compile. To keep timed-lock builds working, either keep std::condition_variable_any here, or refactor to use a separate nano::mutex/nano::condition_variable pair for the thread wait (and only take the std::shared_mutex when touching elections).
…s concurrently erased
73c793d to
7ab66ec
Compare
Fixes intermittent test error:
The race occurs in
election::replace_by_weight():Summary:
replace_by_weight(), unlocks the election mutex at line 772erase_election(), takes a snapshot ofelection->blocks()vote_router.disconnect(election)which erases all blocks including the one Thread A will try to disconnectvote_router.disconnect(replaced_block)- assertion fails becauseerased == 0