Page MenuHomePhabricator

Require no cs_main lock for ProcessNewBlock/ActivateBestChain
ClosedPublic

Authored by Fabien on Feb 26 2019, 11:01.

Details

Summary

This requires the removal of some very liberal (incorrect) cs_mains
sprinkled in some tests. It adds some chainActive.Tip() races, but
the tests are all single-threaded anyway.

Depends on D2607, D2617 and D2631

Partial backport of core PR11824 (commit a99b76f)

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Feb 26 2019, 11:01
Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 26 2019, 11:01
Herald added a subscriber: schancel. · View Herald Transcript
Fabien planned changes to this revision.Feb 26 2019, 17:22
jasonbcox added inline comments.
src/test/miner_tests.cpp
298 ↗(On Diff #7482)

I know this has changes planned, but just pointing out that you got some tabs in here.

Fabien added inline comments.Feb 27 2019, 07:43
src/test/miner_tests.cpp
298 ↗(On Diff #7482)

The planned changes are due to a missing dependency.
These tabs are normal (intended they are spaces and not real \t chars), there is an added bracket pair to limit the scope of the LOCK(cs_main). The closing bracket is just below.

Fabien edited the summary of this revision. (Show Details)Feb 27 2019, 08:23
Fabien edited the summary of this revision. (Show Details)Feb 27 2019, 08:28
Fabien updated this revision to Diff 7502.Feb 27 2019, 08:34

Rebase

Fabien edited the summary of this revision. (Show Details)Feb 27 2019, 08:34
deadalnix requested changes to this revision.Feb 28 2019, 14:38
deadalnix added inline comments.
src/test/txvalidationcache_tests.cpp
67 ↗(On Diff #7502)

The patch you back-port removes this.

src/wallet/test/wallet_tests.cpp
748 ↗(On Diff #7502)

It seems like there is some other backport missing here, because this doesn't match what's in the backported diff.

This revision now requires changes to proceed.Feb 28 2019, 14:38
Fabien updated this revision to Diff 7527.Feb 28 2019, 16:30

Rebase on top of D2631 and remove missed lock

deadalnix accepted this revision.Feb 28 2019, 16:36
This revision is now accepted and ready to land.Feb 28 2019, 16:36
This revision was automatically updated to reflect the committed changes.