Paths

Table of Contentst

Differential D13037

Require IsBlockPruned() to hold mutex cs_main
ClosedPublic
Actions

Authored by PiRK on Jan 24 2023, 07:19.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABCbd5235a5a860: Require IsBlockPruned() to hold mutex cs_main

Summary

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

This is a partial backport of core#22932
https://github.com/bitcoin/bitcoin/pull/22932/commits/eaeeb88768db529b5241ccd42f1e87579908b4df

Depends on D13036

Test Plan

With clang and DEBUG:

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21830
Build 43295: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 43294: arc lint + arc unit

Event Timeline

PiRK created this revision.Jan 24 2023, 07:19

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 24 2023, 07:19

PiRK requested review of this revision.Jan 24 2023, 07:19

teamcity edited the summary of this revision. (Show Details)Jan 24 2023, 07:19

PiRK added a child revision: D13038: Guard CBlockIndex::nStatus/nFile/nDataPos/nUndoPos by cs_main.Jan 24 2023, 07:22

Harbormaster completed remote builds in B21830: Diff 37655.Jan 24 2023, 08:10

Fabien accepted this revision.Jan 24 2023, 08:55

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/rpc/blockchain.cpp
177	You should move the call to UndoReadFromDisk out of the lock, because it creates recursive locking. cs_main is a recursive lock so this is not an actual issue but better avoid double locks if possible. Not blocking on this because it's not wrong, but can you please check if it's been done by core and eventually backport the change as a follow up ?
1077	Do you need the lock for this one ?
1097	dito

This revision is now accepted and ready to land.Jan 24 2023, 08:55

PiRK added inline comments.Jan 24 2023, 09:11

src/rpc/blockchain.cpp
177	Good remark. It is fixed in core#26308
1077	No. The lock scope is reduced in https://github.com/bitcoin/bitcoin/pull/26308/commits

Closed by commit rABCbd5235a5a860: Require IsBlockPruned() to hold mutex cs_main (authored by Jon Atack <jon@atack.com>, committed by PiRK). · Explain WhyJan 24 2023, 09:21

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABCbd5235a5a860: Require IsBlockPruned() to hold mutex cs_main.

Revision Contents
Changeset List

Path

Size

src/

node/

3 lines

blockstorage.cpp

1 line

rpc/

13 lines

Diff 37655

src/node/blockstorage.h

Loading...

src/node/blockstorage.cpp

Loading...

src/rpc/blockchain.cpp

Loading...