Page MenuHomePhabricator

[thread safety] prevent double lock of cs_main in calls to CChainState::UnwindBlock
ClosedPublic

Authored by majcosta on Jul 16 2020, 20:40.

Details

Summary

In upcoming backport D6940 InvalidateBlock(…) requires mutex cs_main not to be held. Currently we are requiring FinalizeBlock
to be called while cs_main is locked, leading to duplicate calls to LOCK(cs_main), this diff changes that.

Depends on D7203

Note to reviewer: feedback on this diff has been addressed by upcoming D7204. if preferable, they could be squashed together as to avoid any revision from having potential funny locking behavior in FinalizeBlock

Test Plan
cmake .. -GNinja -DCMAKE_BUILD_TYPE=Debug
ninja check && ninja check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix requested changes to this revision.Jul 16 2020, 23:15
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/validation.cpp
2911 ↗(On Diff #22315)

Use assignments for these. these curly bracket are ugly and don't serve any purpose here.

2939 ↗(On Diff #22315)

It seems like changes here in between the finalization and the invalidation could lead to very nasty results.

IIRC, core intrduced a new mutex for the chainstate. You need to be using it n lieu of cs_main.

This revision now requires changes to proceed.Jul 16 2020, 23:15
majcosta edited the summary of this revision. (Show Details)
majcosta edited the summary of this revision. (Show Details)
majcosta edited the summary of this revision. (Show Details)

Accepted, granted you land the next one shortly.

This revision is now accepted and ready to land.Aug 17 2020, 22:17