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

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

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABC5d825d4abb76: [thread safety] prevent double lock of cs_main in calls to CChainState…

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

majcosta created this revision.Jul 16 2020, 20:40

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 16 2020, 20:40

majcosta requested review of this revision.Jul 16 2020, 20:40

Harbormaster completed remote builds in B12034: Diff 22315.Jul 16 2020, 20:54

majcosta mentioned this in D6940: [backport#15971] validation: Add compile-time checking for negative locking requirement in LimitValidationInterfaceQueue.Jul 16 2020, 21:00

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.