Page MenuHomePhabricator
Differential D6259

[backport#15855] [refactor] interfaces: Add missing LockAnnotation for cs_main
ClosedPublic
Actions

Authored by majcosta on May 26 2020, 15:15.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC0ae0f30d6b9f: [backport#15855] [refactor] interfaces: Add missing LockAnnotation for cs_main
Summary

https://github.com/bitcoin/bitcoin/pull/15855/commits/fa3c6511435149782545ac0d09d4722dc115d709

Depends on D6260

Backport of Core PR15855

Test Plan
cmake .. -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DENABLE_WERROR=ON -DENABLE_SANITIZERS=thread
ninja check-all
ninja
./src/bench/bitcoin-bench

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

majcosta created this revision.May 26 2020, 15:15
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 26 2020, 15:15
majcosta requested review of this revision.May 26 2020, 15:15
teamcity edited the summary of this revision. (Show Details)May 26 2020, 15:15

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

teamcity added a comment.May 26 2020, 15:16

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

teamcity added a comment.May 26 2020, 15:16

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

teamcity added a comment.May 26 2020, 15:16

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

teamcity added a comment.May 26 2020, 15:17
Build Bitcoin ABC Diffs / Diff Testing (build-werror) failed.

Snippet of first build failure:

[15:16:48] :	 [Step 1/2] [237/459] Building CXX object src/CMakeFiles/common.dir/versionbitsinfo.cpp.o
[15:16:48] :	 [Step 1/2] [238/459] Building CXX object src/CMakeFiles/script.dir/script/script.cpp.o
[15:16:48] :	 [Step 1/2] [239/459] Building CXX object src/CMakeFiles/common.dir/netbase.cpp.o
[15:16:48] :	 [Step 1/2] [240/459] Building CXX object src/CMakeFiles/common.dir/psbt.cpp.o
[15:16:48] :	 [Step 1/2] [241/459] Building CXX object src/CMakeFiles/common.dir/rpc/util.cpp.o
[15:16:48] :	 [Step 1/2] [242/459] Building CXX object src/CMakeFiles/bitcoind.dir/bitcoind.cpp.o
[15:16:48] :	 [Step 1/2] [243/459] Building CXX object src/CMakeFiles/bitcoinconsensus-shared.dir/script/bitcoinconsensus.cpp.o
[15:16:48] :	 [Step 1/2] [244/459] Building CXX object src/CMakeFiles/common.dir/scheduler.cpp.o
[15:16:48] :	 [Step 1/2] [245/459] Building CXX object src/CMakeFiles/common.dir/warnings.cpp.o
[15:16:48] :	 [Step 1/2] [246/459] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[15:16:48] :	 [Step 1/2] [247/459] Building CXX object src/CMakeFiles/server.dir/addrdb.cpp.o
[15:16:48] :	 [Step 1/2] [248/459] Building CXX object src/CMakeFiles/server.dir/addrman.cpp.o
[15:16:48] :	 [Step 1/2] [249/459] Building CXX object src/CMakeFiles/server.dir/blockfilter.cpp.o
[15:16:48] :	 [Step 1/2] [250/459] Building CXX object src/CMakeFiles/common.dir/consensus/merkle.cpp.o
[15:16:48] :	 [Step 1/2] [251/459] Building CXX object src/CMakeFiles/server.dir/banman.cpp.o
[15:16:48] :	 [Step 1/2] [252/459] Building CXX object src/CMakeFiles/server.dir/config.cpp.o
[15:16:48] :	 [Step 1/2] [253/459] Building CXX object src/CMakeFiles/server.dir/consensus/tx_check.cpp.o
[15:16:48] :	 [Step 1/2] [254/459] Building CXX object src/CMakeFiles/server.dir/dbwrapper.cpp.o
[15:16:48] :	 [Step 1/2] [255/459] Building CXX object src/CMakeFiles/server.dir/flatfile.cpp.o
[15:16:48] :	 [Step 1/2] [256/459] Building CXX object src/CMakeFiles/server.dir/httpserver.cpp.o
[15:16:49] :	 [Step 1/2] [257/459] Building CXX object src/CMakeFiles/common.dir/eventloop.cpp.o
[15:16:49] :	 [Step 1/2] [258/459] Building CXX object src/CMakeFiles/script.dir/script/descriptor.cpp.o
[15:16:49] :	 [Step 1/2] [259/459] Building CXX object src/CMakeFiles/common.dir/policy/policy.cpp.o
[15:16:49] :	 [Step 1/2] [260/459] Building CXX object src/CMakeFiles/server.dir/httprpc.cpp.o
[15:16:50] :	 [Step 1/2] [261/459] Building CXX object src/CMakeFiles/server.dir/checkpoints.cpp.o
[15:16:50] :	 [Step 1/2] FAILED: src/CMakeFiles/server.dir/checkpoints.cpp.o 
[15:16:50] :	 [Step 1/2] /usr/bin/ccache /usr/bin/clang++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -I../src/leveldb/helpers/memenv -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/miniupnpc -g -O2 -fPIC -fvisibility=hidden   -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wformat-security -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety-analysis -Wshadow -Wrange-loop-analysis -Wredundant-decls -Wredundant-move -Wno-unused-parameter -Wno-implicit-fallthrough -Werror=return-type -Werror=switch -Werror=thread-safety-analysis -Werror=vla -pthread -std=gnu++14 -MD -MT src/CMakeFiles/server.dir/checkpoints.cpp.o -MF src/CMakeFiles/server.dir/checkpoints.cpp.o.d -o src/CMakeFiles/server.dir/checkpoints.cpp.o -c ../src/checkpoints.cpp
[15:16:50] :	 [Step 1/2] ../src/checkpoints.cpp:31:31: error: calling function 'LookupBlockIndex' requires holding mutex 'cs_main' exclusively [-Werror,-Wthread-safety-analysis]
[15:16:50] :	 [Step 1/2]         CBlockIndex *pindex = LookupBlockIndex(hash);
[15:16:50] :	 [Step 1/2]                               ^
[15:16:50] :	 [Step 1/2] 1 error generated.
[15:16:50] :	 [Step 1/2] [262/459] Building CXX object src/CMakeFiles/server.dir/chain.cpp.o
[15:16:50] :	 [Step 1/2] [263/459] Building CXX object src/CMakeFiles/server.dir/consensus/activation.cpp.o
[15:16:50] :	 [Step 1/2] [264/459] Building CXX object src/CMakeFiles/server.dir/consensus/tx_verify.cpp.o
[15:16:52] :	 [Step 1/2] [265/459] Building CXX object src/CMakeFiles/server.dir/index/base.cpp.o
[15:16:52] :	 [Step 1/2] [266/459] Building CXX object src/CMakeFiles/server.dir/minerfund.cpp.o
[15:16:53] :	 [Step 1/2] [267/459] Building CXX object src/CMakeFiles/server.dir/interfaces/chain.cpp.o
[15:16:53] :	 [Step 1/2] FAILED: src/CMakeFiles/server.dir/interfaces/chain.cpp.o 
[15:16:53] :	 [Step 1/2] /usr/bin/ccache /usr/bin/clang++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -I../src/leveldb/helpers/memenv -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/miniupnpc -g -O2 -fPIC -fvisibility=hidden   -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wformat-security -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety-analysis -Wshadow -Wrange-loop-analysis -Wredundant-decls -Wredundant-move -Wno-unused-parameter -Wno-implicit-fallthrough -Werror=return-type -Werror=switch -Werror=thread-safety-analysis -Werror=vla -pthread -std=gnu++14 -MD -MT src/CMakeFiles/server.dir/interfaces/chain.cpp.o -MF src/CMakeFiles/server.dir/interfaces/chain.cpp.o.d -o src/CMakeFiles/server.dir/interfaces/chain.cpp.o -c ../src/interfaces/chain.cpp
[15:16:53] :	 [Step 1/2] ../src/interfaces/chain.cpp:135:34: error: calling function 'LookupBlockIndex' requires holding mutex 'cs_main' exclusively [-Werror,-Wthread-safety-analysis]
[15:16:53] :	 [Step 1/2]             CBlockIndex *block = LookupBlockIndex(hash);
[15:16:53] :	 [Step 1/2]                                  ^
[15:16:53] :	 [Step 1/2] 1 error generated.
[15:16:53] :	 [Step 1/2] [268/459] Building CXX object src/CMakeFiles/common.dir/rpc/rawtransaction_util.cpp.o
[15:16:53] :	 [Step 1/2] [269/459] Building CXX object src/CMakeFiles/server.dir/blockencodings.cpp.o
[15:16:54] :	 [Step 1/2] [270/459] Building CXX object src/CMakeFiles/server.dir/index/txindex.cpp.o
[15:16:55] :	 [Step 1/2] [271/459] Building CXX object src/CMakeFiles/server.dir/interfaces/node.cpp.o
[15:16:55] :	 [Step 1/2] [272/459] Building CXX object src/CMakeFiles/server.dir/avalanche.cpp.o
[15:16:55] :	 [Step 1/2] [273/459] Building CXX object src/CMakeFiles/server.dir/miner.cpp.o
[15:16:58] :	 [Step 1/2] [274/459] Building CXX object src/CMakeFiles/server.dir/init.cpp.o
[15:16:58] :	 [Step 1/2] ninja: build stopped: subcommand failed.
[15:16:58]W:	 [Step 1/2] ++ print_sanitizers_log
[15:16:58] :	 [Step 1/2] *** Output of /tmp/sanitizer_logs/*.log.* ***
[15:16:58]W:	 [Step 1/2] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[15:16:58]W:	 [Step 1/2] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[15:16:58]W:	 [Step 1/2] ++ cat '/tmp/sanitizer_logs/*.log.*'
[15:16:58]W:	 [Step 1/2] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[15:16:59]W:	 [Step 1/2] cp: cannot stat '/work/ibd/debug.log': No such file or directory
[15:17:02]W:	 [Step 1/2] Process exited with code 1
[15:17:02]E:	 [Step 1/2] Process exited with code 1 (Step: Command Line)
Harbormaster completed remote builds in B11030: Diff 20617.May 26 2020, 15:17
teamcity added a comment.May 26 2020, 15:20

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

teamcity added a comment.May 26 2020, 15:26

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

majcosta planned changes to this revision.May 26 2020, 15:33
majcosta updated this revision to Diff 20619.EditedMay 26 2020, 17:39

rebased onto [backport#15842]

teamcity added a comment.May 26 2020, 17:39

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

teamcity added a comment.May 26 2020, 17:39

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

teamcity added a comment.May 26 2020, 17:39

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

majcosta edited the summary of this revision. (Show Details)May 26 2020, 17:39
majcosta added a parent revision: D6260: [backport#15842] refactor: replace isPotentialtip/waitForNotifications by higher method.
teamcity added a comment.May 26 2020, 17:41

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

teamcity added a comment.May 26 2020, 17:43

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Harbormaster completed remote builds in B11032: Diff 20619.May 26 2020, 17:51
teamcity added a comment.May 26 2020, 17:51

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

majcosta updated this revision to Diff 20625.May 26 2020, 18:11

rebase

teamcity added a comment.May 26 2020, 18:16

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

teamcity added a comment.May 26 2020, 18:16

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

teamcity added a comment.May 26 2020, 18:16

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

teamcity added a comment.May 26 2020, 18:19

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

teamcity added a comment.May 26 2020, 18:20

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

teamcity added a comment.May 26 2020, 18:26

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B11038: Diff 20625.May 26 2020, 18:26
majcosta added a child revision: D6261: [backport#16034] Move LockAnnotation from threadsafety.h (imported code) to sync.h (our code).May 26 2020, 20:06
deadalnix requested changes to this revision.May 27 2020, 11:45
deadalnix added a subscriber: deadalnix.

Test plan is not appropriate.

This revision now requires changes to proceed.May 27 2020, 11:45
majcosta requested review of this revision.May 27 2020, 18:59
majcosta edited the test plan for this revision. (Show Details)
majcosta edited the test plan for this revision. (Show Details)

expanded test plan

deadalnix accepted this revision.May 27 2020, 19:20
This revision is now accepted and ready to land.May 27 2020, 19:20
Closed by commit rABC0ae0f30d6b9f: [backport#15855] [refactor] interfaces: Add missing LockAnnotation for cs_main (authored by MarcoFalke <falke.marco@gmail.com>, committed by majcosta). · Explain WhyMay 27 2020, 19:33
This revision was automatically updated to reflect the committed changes.
majcosta added a commit: rABC0ae0f30d6b9f: [backport#15855] [refactor] interfaces: Add missing LockAnnotation for cs_main.

Revision Contents
Changeset List

PathSize
src/
bench/
1 line
3 lines
3 lines
interfaces/
12 lines
qt/
test/
2 lines
test/
3 lines
20 lines
1 line
1 line
10 lines
wallet/
test/
20 lines

Diff 20652

View Options

src/bench/duplicate_inputs.cpp

Loading...
View Options

src/chain.h

Loading...
View Options

src/checkpoints.cpp

Loading...
View Options

src/interfaces/chain.cpp

Loading...
View Options

src/qt/test/wallettests.cpp

Loading...
View Options

src/test/miner_tests.cpp

Loading...
View Options

src/test/txvalidationcache_tests.cpp

Loading...
View Options

src/test/util.cpp

Loading...
View Options

src/test/validation_block_tests.cpp

Loading...
View Options

src/validation.cpp

Loading...
View Options

src/wallet/test/wallet_tests.cpp

Loading...