- Add missing
cs_main
lock required when accessingpblocktree
. Add annotation. - Add missing
cs_main
lock required when accessingpcoinsTip
. Add annotation. - Add missing
cs_main
lock required when accessingpcoinsdbview
. Add annotation. - Add missing
cs_main
lock required when accessingscriptExecutionCache
. Add annotation.
Add missing cs_main locks in ThreadImport(…)/Shutdown(…)/gettxoutsetinfo(…)/InitScriptExecutionCache(). Add missing locking annotations. #15192
pull practicalswift wants to merge 4 commits into bitcoin:master from practicalswift:validation-cs_main changing 4 files +22 −10-
practicalswift commented at 9:29 pm on January 17, 2019: contributor
-
practicalswift renamed this:
Add missing cs_main locks in ThreadImport(...)/Shutdown(...)/gettxoutsetinfo(...)/InitScriptExecutionCache() and benchmarks/tests. Add annotations.
Add missing cs_main locks in ThreadImport(...)/Shutdown(...)/gettxoutsetinfo(...)/InitScriptExecutionCache() and benchmarks/tests. Add missing locking annotations.
on Jan 17, 2019 -
DrahtBot commented at 11:33 pm on January 17, 2019: member
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #16443 (refactor: have CCoins* data managed under CChainState by jamesob)
- #15997 (refactor: GuessVerificationProgress requires cs_main lock by promag)
- #13204 (Faster sigcache nonce by JeremyRubin)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
-
in src/bench/block_assemble.cpp:76 in 01b4b0bc2c outdated
72@@ -73,9 +73,12 @@ static void AssembleBlock(benchmark::State& state) 73 boost::thread_group thread_group; 74 CScheduler scheduler; 75 { 76- ::pblocktree.reset(new CBlockTreeDB(1 << 20, true)); 77- ::pcoinsdbview.reset(new CCoinsViewDB(1 << 23, true)); 78- ::pcoinsTip.reset(new CCoinsViewCache(pcoinsdbview.get())); 79+ {
MarcoFalke commented at 4:01 am on January 25, 2019:style-nit: I created the above scope to prevent “leaking” of variable names further down, so no need to nest scopes here, just close/open the existing one.
practicalswift commented at 9:57 am on January 25, 2019:Fixed! -
practicalswift force-pushed on Jan 25, 2019
-
practicalswift commented at 9:57 am on January 25, 2019: contributor@MarcoFalke Thanks for reviewing. Feedback addressed! Please re-review :-)
-
practicalswift commented at 4:48 pm on January 31, 2019: contributor@MarcoFalke Would it be possible to get a release milestone for this PR? :-)
-
MarcoFalke added this to the milestone 0.18.0 on Jan 31, 2019
-
in src/validation.cpp:3620 in fbb548ff38 outdated
3625@@ -3625,7 +3626,7 @@ uint64_t CalculateCurrentUsage() 3626 } 3627 3628 /* Prune a block file (modify associated database entries)*/ 3629-void PruneOneBlockFile(const int fileNumber) 3630+void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
MarcoFalke commented at 6:29 pm on February 6, 2019:Needs this commit removed -
practicalswift force-pushed on Feb 6, 2019
-
practicalswift force-pushed on Feb 7, 2019
-
practicalswift commented at 9:53 pm on February 7, 2019: contributor@MarcoFalke Fixed! Please re-review :-)
-
MarcoFalke removed this from the milestone 0.18.0 on Feb 11, 2019
-
practicalswift commented at 6:55 am on February 12, 2019: contributor@MarcoFalke What is the reason for the removal of the 0.18.0 milestone?
-
MarcoFalke commented at 4:21 pm on February 12, 2019: memberIt couldn’t be merged without review, and that seems not trivial in some of the hunks
-
MarcoFalke commented at 4:23 pm on February 12, 2019: memberanything in src/test and src/bench looks good and can be merged
-
MarcoFalke referenced this in commit 4d2767c228 on Feb 15, 2019
-
practicalswift force-pushed on Feb 15, 2019
-
practicalswift commented at 12:55 pm on February 15, 2019: contributor
@MarcoFalke Thanks for the quick turnaround with regards to the test/bench split-up PR. I’ve now rebased this one on top of
master
.Regarding the remaining changes: perhaps I should exclude any non-trivial changes (by opting out) so that we get proper annotations (
GUARDED_BY
). That will help guard against future missed locks which is one step forward :-) -
practicalswift commented at 1:10 pm on February 15, 2019: contributor@MarcoFalke Is it the added lock
InitScriptExecutionCache
you feel could be non-trivial to review? If so I can just skip it :-) -
practicalswift renamed this:
Add missing cs_main locks in ThreadImport(...)/Shutdown(...)/gettxoutsetinfo(...)/InitScriptExecutionCache() and benchmarks/tests. Add missing locking annotations.
Add missing cs_main locks in ThreadImport(...)/Shutdown(...)/gettxoutsetinfo(...)/InitScriptExecutionCache(). Add missing locking annotations.
on Mar 4, 2019 -
DrahtBot added the label RPC/REST/ZMQ on Mar 19, 2019
-
DrahtBot added the label Validation on Mar 19, 2019
-
DrahtBot added the label Needs rebase on Jun 5, 2019
-
practicalswift force-pushed on Jun 5, 2019
-
practicalswift commented at 11:22 pm on June 5, 2019: contributorRebased!
-
DrahtBot removed the label Needs rebase on Jun 5, 2019
-
practicalswift force-pushed on Jun 5, 2019
-
practicalswift commented at 5:55 pm on June 26, 2019: contributor@MarcoFalke Should this locking PR be closed? Please advice.
-
MarcoFalke commented at 6:15 pm on June 26, 2019: memberConcept ACK
-
MarcoFalke added this to the milestone 0.19.0 on Jun 26, 2019
-
practicalswift force-pushed on Jun 26, 2019
-
DrahtBot added the label Needs rebase on Jul 3, 2019
-
Add missing cs_main lock required when accessing scriptExecutionCache. Add annotation. b093101fcc
-
Add missing cs_main lock required when accessing pcoinsdbview. Add annotation. 35cb3a5915
-
Add missing cs_main lock required when accessing pcoinsTip. Add annotation. 0171a1b2cf
-
Add missing cs_main lock required when accessing pblocktree. Add annotation. b6ba1832d8
-
practicalswift force-pushed on Jul 3, 2019
-
DrahtBot removed the label Needs rebase on Jul 4, 2019
-
practicalswift closed this on Aug 10, 2019
-
practicalswift deleted the branch on Apr 10, 2021
-
Munkybooty referenced this in commit dc8cd6897b on Sep 5, 2021
-
Munkybooty referenced this in commit 8398a6750c on Sep 7, 2021
-
Munkybooty referenced this in commit 9b00ccea66 on Sep 7, 2021
-
Munkybooty referenced this in commit 9a157896f1 on Sep 9, 2021
-
Munkybooty referenced this in commit 4f390c3fb4 on Sep 10, 2021
-
Munkybooty referenced this in commit 9ec6b6e64a on Sep 13, 2021
-
Munkybooty referenced this in commit 06925a1464 on Sep 13, 2021
-
DrahtBot locked this on Aug 18, 2022