Conversation
Network workers, synchronization tasks, metrics collectors, and backup services access several mutable states across threads without consistent visibility or synchronization guarantees. Use concurrent collections and synchronized state transitions where compound access must remain atomic, publish cross-thread fields safely, serialize peer cleanup with existing manager operations, and prevent duplicate-witness metrics from exposing incomplete companion state.
7442a73 to
19681e4
Compare
| && peer.getSyncBlockToFetch().size() > syncFetchBatchNum)) { | ||
| syncService.setFetchFlag(true); | ||
| } else { | ||
| syncService.syncNext(peer); |
There was a problem hiding this comment.
[MUST] Holding blockLock only on this call path does not make the check-then-set in syncNext() atomic; SyncService.processBlock() still calls syncNext() without that lock. Two threads can both observe syncChainRequested == null and send requests, after which a valid second response may be treated as BAD_MESSAGE. Put the check, summary creation, state update, and send under a lock shared by every caller, and add a same-peer concurrency test.
| .initialCapacity(200).maximumSize(200).build(); | ||
|
|
||
| private Map<String, CheatWitnessInfo> cheatWitnessInfoMap = new HashMap<>(); | ||
| private Map<String, CheatWitnessInfo> cheatWitnessInfoMap = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
[MUST] Replacing the outer map with ConcurrentHashMap only makes individual map operations safe. The cache get→put at lines 27–39 can let two conflicting blocks for the same witness and height both observe null and overwrite each other; containsKey→put→clear/add can also lose counts, while NodeInfoService may iterate the inner HashSet concurrently. Use one atomic section for detection and update, expose an immutable or synchronized snapshot to readers, and add a real concurrency test.
3 participants
What does this PR do?
Network workers, synchronization tasks, metrics collectors, and backup services access several mutable states across threads without consistent visibility or synchronization guarantees.
Use concurrent collections and synchronized state transitions where compound access must remain atomic, publish cross-thread fields safely, serialize peer cleanup with existing manager operations, and prevent duplicate-witness metrics from exposing incomplete companion state.
Fixes #6932
Why are these changes required?
This PR has been tested by:
Follow up
Extra details