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
Branch
PR11824_part4
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5110
Build 8283: Bitcoin ABC Buildbot (legacy)
Build 8282: arc lint + arc unit

Event Timeline

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.

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.

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

Rebase on top of D2631 and remove missed lock

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