Page MenuHomePhabricator
Differential D6539

[backport#14193] validation: Add missing mempool locks
ClosedPublic
Actions

Authored by majcosta on Jun 11 2020, 17:09.

Details

Reviewers
Fabien
deadalnix
Group Reviewers
Restricted Project
Commits
rABC472a67420336: [backport#14193] validation: Add missing mempool locks
Summary

https://github.com/bitcoin/bitcoin/pull/14193/commits/fabeb1f613653a8c1560e4a093a9b6b7a069b60b

Depends on D6537

Partial backport of Core PR14193

Test Plan
cmake .. -DCMAKE_BUILD_TYPE=Debug
ninja check-all

Diff Detail

Event Timeline

majcosta created this revision.Jun 11 2020, 17:09
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 11 2020, 17:09
majcosta requested review of this revision.Jun 11 2020, 17:09
teamcity edited the summary of this revision. (Show Details)Jun 11 2020, 17:10

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

Harbormaster completed remote builds in B11430: Diff 21298.Jun 11 2020, 17:11
Fabien requested changes to this revision.Jun 11 2020, 18:44
Fabien added a subscriber: Fabien.

Clang build is unhappy

This revision now requires changes to proceed.Jun 11 2020, 18:44
majcosta updated this revision to Diff 21312.Jun 12 2020, 01:11

added a missing lock in DisconnectedBlockTransactions::addForBlock

teamcity added a comment.Jun 12 2020, 01:14
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Snippet of first build failure:

[01:13:39] :	 [Step 1/2] [362/412] Generating forms/ui_overviewpage.h
[01:13:39] :	 [Step 1/2] [363/412] Generating forms/ui_receivecoinsdialog.h
[01:13:39] :	 [Step 1/2] [364/412] Generating forms/ui_sendcoinsdialog.h
[01:13:39] :	 [Step 1/2] [365/412] Generating forms/ui_sendcoinsentry.h
[01:13:39] :	 [Step 1/2] [366/412] Generating forms/ui_signverifymessagedialog.h
[01:13:39] :	 [Step 1/2] [367/412] Generating temp_bitcoin_locale.qrc
[01:13:39] :	 [Step 1/2] [368/412] Generating qrc_bitcoin.cpp
[01:13:40] :	 [Step 1/2] [369/412] Generating qrc_bitcoin_locale.cpp
[01:13:41] :	 [Step 1/2] [370/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinaddressvalidator.cpp.o
[01:13:41] :	 [Step 1/2] [371/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/bench_bitcoin.cpp.o
[01:13:43] :	 [Step 1/2] [372/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_mempool.cpp.o
[01:13:43] :	 [Step 1/2] [373/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/crypto_aes.cpp.o
[01:13:44] :	 [Step 1/2] [374/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinunits.cpp.o
[01:13:44] :	 [Step 1/2] [375/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/mempool_eviction.cpp.o
[01:13:44] :	 [Step 1/2] [376/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_blockchain.cpp.o
[01:13:44] :	 [Step 1/2] [377/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/checkblock.cpp.o
[01:13:44] :	 [Step 1/2] [378/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/duplicate_inputs.cpp.o
[01:13:44] :	 [Step 1/2] [379/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/csvmodelwriter.cpp.o
[01:13:44] :	 [Step 1/2] [380/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/block_assemble.cpp.o
[01:13:44] :	 [Step 1/2] [381/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/networkstyle.cpp.o
[01:13:44] :	 [Step 1/2] [382/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinamountfield.cpp.o
[01:13:45] :	 [Step 1/2] [383/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/__/test/util.cpp.o
[01:13:45] :	 [Step 1/2] [384/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bantablemodel.cpp.o
[01:13:45] :	 [Step 1/2] [385/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/platformstyle.cpp.o
[01:13:47] :	 [Step 1/2] [386/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o
[01:13:47] :	 [Step 1/2] FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o 
[01:13:47] :	 [Step 1/2] /usr/bin/ccache /usr/bin/c++  -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 -DHAVE_DECL_EVP_MD_CTX_NEW=1 -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DUSE_PROTOBUF_MESSAGE_BYTESIZELONG -Isrc/qt/bitcoin-qt-base_autogen/include -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -fPIC -std=gnu++14 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o -c ../src/qt/bitcoingui.cpp
[01:13:47] :	 [Step 1/2] ../src/qt/bitcoingui.cpp: In member function 'void BitcoinGUI::updateProxyIcon()':
[01:13:47] :	 [Step 1/2] ../src/qt/bitcoingui.cpp:1364:40: error: 'ReturnByValue' is not a member of 'Qt'
[01:13:47] :	 [Step 1/2]          if (labelProxyIcon->pixmap(Qt::ReturnByValue).isNull()) {
[01:13:47] :	 [Step 1/2]                                         ^~~~~~~~~~~~~
[01:13:47] :	 [Step 1/2] [387/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qvaluecombobox.cpp.o
[01:13:47] :	 [Step 1/2] [388/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qvalidatedlineedit.cpp.o
[01:13:48] :	 [Step 1/2] [389/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/notificator.cpp.o
[01:13:48] :	 [Step 1/2] [390/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/modaloverlay.cpp.o
[01:13:48] :	 [Step 1/2] [391/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoin.cpp.o
[01:13:49] :	 [Step 1/2] [392/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/clientmodel.cpp.o
[01:13:50] :	 [Step 1/2] [393/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/intro.cpp.o
[01:13:50] :	 [Step 1/2] [394/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o
[01:13:50] :	 [Step 1/2] FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o 
[01:13:50] :	 [Step 1/2] /usr/bin/ccache /usr/bin/c++  -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 -DHAVE_DECL_EVP_MD_CTX_NEW=1 -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DUSE_PROTOBUF_MESSAGE_BYTESIZELONG -Isrc/qt/bitcoin-qt-base_autogen/include -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -fPIC -std=gnu++14 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o -c ../src/qt/optionsmodel.cpp
[01:13:50] :	 [Step 1/2] ../src/qt/optionsmodel.cpp: In function 'ProxySetting GetProxySetting(QSettings&, const QString&)':
[01:13:50] :	 [Step 1/2] ../src/qt/optionsmodel.cpp:273:56: error: 'SkipEmptyParts' is not a member of 'Qt'
[01:13:50] :	 [Step 1/2]          settings.value(name).toString().split(":", Qt::SkipEmptyParts);
[01:13:50] :	 [Step 1/2]                                                         ^~~~~~~~~~~~~~
[01:13:50] :	 [Step 1/2] [395/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/__/test/util/setup_common.cpp.o
[01:13:51] :	 [Step 1/2] [396/412] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/bench.cpp.o
[01:13:52] :	 [Step 1/2] [397/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsdialog.cpp.o
[01:13:52] :	 [Step 1/2] [398/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/guiutil.cpp.o
[01:13:52] :	 [Step 1/2] [399/412] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/peertablemodel.cpp.o
[01:13:52] :	 [Step 1/2] ninja: build stopped: subcommand failed.
[01:13:52]W:	 [Step 1/2] ++ print_sanitizers_log
[01:13:52]W:	 [Step 1/2] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[01:13:52]W:	 [Step 1/2] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[01:13:52]W:	 [Step 1/2] ++ cat '/tmp/sanitizer_logs/*.log.*'
[01:13:52] :	 [Step 1/2] *** Output of /tmp/sanitizer_logs/*.log.* ***
[01:13:52]W:	 [Step 1/2] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[01:13:53]W:	 [Step 1/2] cp: cannot stat '/work/ibd/debug.log': No such file or directory
[01:14:02]W:	 [Step 1/2] Process exited with code 1
[01:14:02]E:	 [Step 1/2] Process exited with code 1 (Step: Command Line)
Harbormaster completed remote builds in B11437: Diff 21312.Jun 12 2020, 01:14
teamcity added a comment.Jun 12 2020, 01:14
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Snippet of first build failure:

[01:14:02] :	 [Step 1/2] [392/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/crypto_aes.cpp.o
[01:14:02] :	 [Step 1/2] [393/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_mempool.cpp.o
[01:14:03] :	 [Step 1/2] [394/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/block_assemble.cpp.o
[01:14:03] :	 [Step 1/2] [395/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/duplicate_inputs.cpp.o
[01:14:03] :	 [Step 1/2] [396/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/csvmodelwriter.cpp.o
[01:14:03] :	 [Step 1/2] [397/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/mempool_eviction.cpp.o
[01:14:03] :	 [Step 1/2] [398/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_blockchain.cpp.o
[01:14:05] :	 [Step 1/2] [399/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinunits.cpp.o
[01:14:05] :	 [Step 1/2] [400/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinaddressvalidator.cpp.o
[01:14:05] :	 [Step 1/2] [401/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinamountfield.cpp.o
[01:14:06] :	 [Step 1/2] [402/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/__/test/util.cpp.o
[01:14:06] :	 [Step 1/2] [403/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bantablemodel.cpp.o
[01:14:07] :	 [Step 1/2] [404/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/networkstyle.cpp.o
[01:14:07] :	 [Step 1/2] [405/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/platformstyle.cpp.o
[01:14:07] :	 [Step 1/2] [406/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qvalidatedlineedit.cpp.o
[01:14:07] :	 [Step 1/2] [407/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/modaloverlay.cpp.o
[01:14:08] :	 [Step 1/2] [408/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/clientmodel.cpp.o
[01:14:09] :	 [Step 1/2] [409/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/intro.cpp.o
[01:14:09] :	 [Step 1/2] [410/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o
[01:14:09] :	 [Step 1/2] FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o 
[01:14:09] :	 [Step 1/2] /usr/bin/ccache /usr/bin/c++  -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 -DHAVE_DECL_EVP_MD_CTX_NEW=1 -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DUSE_PROTOBUF_MESSAGE_BYTESIZELONG -Isrc/qt/bitcoin-qt-base_autogen/include -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -fPIC -std=gnu++14 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o -c ../src/qt/bitcoingui.cpp
[01:14:09] :	 [Step 1/2] ../src/qt/bitcoingui.cpp: In member function 'void BitcoinGUI::updateProxyIcon()':
[01:14:09] :	 [Step 1/2] ../src/qt/bitcoingui.cpp:1364:40: error: 'ReturnByValue' is not a member of 'Qt'
[01:14:09] :	 [Step 1/2]          if (labelProxyIcon->pixmap(Qt::ReturnByValue).isNull()) {
[01:14:09] :	 [Step 1/2]                                         ^~~~~~~~~~~~~
[01:14:10] :	 [Step 1/2] [411/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qvaluecombobox.cpp.o
[01:14:10] :	 [Step 1/2] [412/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/notificator.cpp.o
[01:14:10] :	 [Step 1/2] [413/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/coin_selection.cpp.o
[01:14:10] :	 [Step 1/2] [414/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/__/test/util/setup_common.cpp.o
[01:14:11] :	 [Step 1/2] [415/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/bench.cpp.o
[01:14:12] :	 [Step 1/2] [416/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoin.cpp.o
[01:14:13] :	 [Step 1/2] [417/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/trafficgraphwidget.cpp.o
[01:14:13] :	 [Step 1/2] [418/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/guiutil.cpp.o
[01:14:13] :	 [Step 1/2] [419/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o
[01:14:13] :	 [Step 1/2] FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o 
[01:14:13] :	 [Step 1/2] /usr/bin/ccache /usr/bin/c++  -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 -DHAVE_DECL_EVP_MD_CTX_NEW=1 -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DUSE_PROTOBUF_MESSAGE_BYTESIZELONG -Isrc/qt/bitcoin-qt-base_autogen/include -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -fPIC -std=gnu++14 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o -c ../src/qt/optionsmodel.cpp
[01:14:13] :	 [Step 1/2] ../src/qt/optionsmodel.cpp: In function 'ProxySetting GetProxySetting(QSettings&, const QString&)':
[01:14:13] :	 [Step 1/2] ../src/qt/optionsmodel.cpp:273:56: error: 'SkipEmptyParts' is not a member of 'Qt'
[01:14:13] :	 [Step 1/2]          settings.value(name).toString().split(":", Qt::SkipEmptyParts);
[01:14:13] :	 [Step 1/2]                                                         ^~~~~~~~~~~~~~
[01:14:14] :	 [Step 1/2] [420/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/splashscreen.cpp.o
[01:14:14] :	 [Step 1/2] [421/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsdialog.cpp.o
[01:14:14] :	 [Step 1/2] [422/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/peertablemodel.cpp.o
[01:14:15] :	 [Step 1/2] [423/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o
[01:14:15] :	 [Step 1/2] FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o 
[01:14:15] :	 [Step 1/2] /usr/bin/ccache /usr/bin/c++  -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 -DHAVE_DECL_EVP_MD_CTX_NEW=1 -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DUSE_PROTOBUF_MESSAGE_BYTESIZELONG -Isrc/qt/bitcoin-qt-base_autogen/include -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -fPIC -std=gnu++14 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o -c ../src/qt/rpcconsole.cpp
[01:14:15] :	 [Step 1/2] ../src/qt/rpcconsole.cpp: In member function 'void RPCConsole::setClientModel(ClientModel*)':
[01:14:15] :	 [Step 1/2] ../src/qt/rpcconsole.cpp:727:47: error: 'mappedInt' is not a member of 'QSignalMapper'
[01:14:15] :	 [Step 1/2]          connect(signalMapper, &QSignalMapper::mappedInt, this,
[01:14:15] :	 [Step 1/2]                                                ^~~~~~~~~
[01:14:15] :	 [Step 1/2] ninja: build stopped: subcommand failed.
[01:14:15]W:	 [Step 1/2] ++ print_sanitizers_log
[01:14:15]W:	 [Step 1/2] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[01:14:15]W:	 [Step 1/2] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[01:14:15]W:	 [Step 1/2] ++ cat '/tmp/sanitizer_logs/*.log.*'
[01:14:15] :	 [Step 1/2] *** Output of /tmp/sanitizer_logs/*.log.* ***
[01:14:15]W:	 [Step 1/2] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[01:14:15]W:	 [Step 1/2] cp: cannot stat '/work/ibd/debug.log': No such file or directory
[01:14:21]W:	 [Step 1/2] Process exited with code 1
[01:14:21]E:	 [Step 1/2] Process exited with code 1 (Step: Command Line)
teamcity added a comment.Jun 12 2020, 01:14
Build Bitcoin ABC Diffs / Diff Testing (build-clang-10) failed.

Snippet of first build failure:

[01:14:11] :	 [Step 1/2] [380/460] Generating forms/ui_overviewpage.h
[01:14:11] :	 [Step 1/2] [381/460] Generating forms/ui_receivecoinsdialog.h
[01:14:11] :	 [Step 1/2] [382/460] Generating forms/ui_receiverequestdialog.h
[01:14:12] :	 [Step 1/2] [383/460] Generating forms/ui_debugwindow.h
[01:14:12] :	 [Step 1/2] [384/460] Generating forms/ui_sendcoinsdialog.h
[01:14:12] :	 [Step 1/2] [385/460] Generating forms/ui_sendcoinsentry.h
[01:14:12] :	 [Step 1/2] [386/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/crypto_aes.cpp.o
[01:14:12] :	 [Step 1/2] [387/460] Generating temp_bitcoin_locale.qrc
[01:14:12] :	 [Step 1/2] [388/460] Generating qrc_bitcoin.cpp
[01:14:12] :	 [Step 1/2] [389/460] Generating qrc_bitcoin_locale.cpp
[01:14:12] :	 [Step 1/2] [390/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/block_assemble.cpp.o
[01:14:12] :	 [Step 1/2] [391/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/checkblock.cpp.o
[01:14:12] :	 [Step 1/2] [392/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/ccoins_caching.cpp.o
[01:14:13] :	 [Step 1/2] [393/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/duplicate_inputs.cpp.o
[01:14:14] :	 [Step 1/2] [394/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/mempool_eviction.cpp.o
[01:14:14] :	 [Step 1/2] [395/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_mempool.cpp.o
[01:14:14] :	 [Step 1/2] [396/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_blockchain.cpp.o
[01:14:15] :	 [Step 1/2] [397/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinunits.cpp.o
[01:14:16] :	 [Step 1/2] [398/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinaddressvalidator.cpp.o
[01:14:16] :	 [Step 1/2] [399/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoinamountfield.cpp.o
[01:14:16] :	 [Step 1/2] [400/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/csvmodelwriter.cpp.o
[01:14:17] :	 [Step 1/2] [401/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bantablemodel.cpp.o
[01:14:18] :	 [Step 1/2] [402/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/__/test/util.cpp.o
[01:14:19] :	 [Step 1/2] [403/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/networkstyle.cpp.o
[01:14:19] :	 [Step 1/2] [404/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o
[01:14:19] :	 [Step 1/2] FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o 
[01:14:19] :	 [Step 1/2] /usr/bin/ccache /usr/bin/clang++-10  -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 -DHAVE_DECL_EVP_MD_CTX_NEW=1 -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DUSE_PROTOBUF_MESSAGE_BYTESIZELONG -Isrc/qt/bitcoin-qt-base_autogen/include -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety-analysis -Wshadow -Wrange-loop-analysis -Wredundant-decls -Wformat-security -Wredundant-move -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -fPIC -std=gnu++14 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o -c ../src/qt/bitcoingui.cpp
[01:14:19] :	 [Step 1/2] ../src/qt/bitcoingui.cpp:1364:40: error: no member named 'ReturnByValue' in namespace 'Qt'
[01:14:19] :	 [Step 1/2]         if (labelProxyIcon->pixmap(Qt::ReturnByValue).isNull()) {
[01:14:19] :	 [Step 1/2]                                    ~~~~^
[01:14:19] :	 [Step 1/2] 1 error generated.
[01:14:20] :	 [Step 1/2] [405/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/clientmodel.cpp.o
[01:14:20] :	 [Step 1/2] [406/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/modaloverlay.cpp.o
[01:14:20] :	 [Step 1/2] [407/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/notificator.cpp.o
[01:14:20] :	 [Step 1/2] [408/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/wallet_balance.cpp.o
[01:14:21] :	 [Step 1/2] [409/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/bench.cpp.o
[01:14:21] :	 [Step 1/2] [410/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/intro.cpp.o
[01:14:21] :	 [Step 1/2] [411/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/__/test/util/setup_common.cpp.o
[01:14:21] :	 [Step 1/2] [412/460] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/coin_selection.cpp.o
[01:14:21] :	 [Step 1/2] [413/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoin.cpp.o
[01:14:22] :	 [Step 1/2] [414/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/guiutil.cpp.o
[01:14:23] :	 [Step 1/2] [415/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/peertablemodel.cpp.o
[01:14:23] :	 [Step 1/2] [416/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o
[01:14:23] :	 [Step 1/2] FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o 
[01:14:23] :	 [Step 1/2] /usr/bin/ccache /usr/bin/clang++-10  -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 -DHAVE_DECL_EVP_MD_CTX_NEW=1 -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DUSE_PROTOBUF_MESSAGE_BYTESIZELONG -Isrc/qt/bitcoin-qt-base_autogen/include -I../src/. -Isrc -I../src/univalue/include -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety-analysis -Wshadow -Wrange-loop-analysis -Wredundant-decls -Wformat-security -Wredundant-move -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -fPIC -std=gnu++14 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o -c ../src/qt/optionsmodel.cpp
[01:14:23] :	 [Step 1/2] ../src/qt/optionsmodel.cpp:273:56: error: no member named 'SkipEmptyParts' in namespace 'Qt'
[01:14:23] :	 [Step 1/2]         settings.value(name).toString().split(":", Qt::SkipEmptyParts);
[01:14:23] :	 [Step 1/2]                                                    ~~~~^
[01:14:23] :	 [Step 1/2] 1 error generated.
[01:14:23] :	 [Step 1/2] [417/460] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsdialog.cpp.o
[01:14:23] :	 [Step 1/2] ninja: build stopped: subcommand failed.
[01:14:23]W:	 [Step 1/2] ++ print_sanitizers_log
[01:14:23]W:	 [Step 1/2] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[01:14:23] :	 [Step 1/2] *** Output of /tmp/sanitizer_logs/*.log.* ***
[01:14:23]W:	 [Step 1/2] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[01:14:23]W:	 [Step 1/2] ++ cat '/tmp/sanitizer_logs/*.log.*'
[01:14:23]W:	 [Step 1/2] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[01:14:24]W:	 [Step 1/2] cp: cannot stat '/work/ibd/debug.log': No such file or directory
[01:14:28]W:	 [Step 1/2] Process exited with code 1
[01:14:28]E:	 [Step 1/2] Process exited with code 1 (Step: Command Line)
majcosta updated this revision to Diff 21313.Jun 12 2020, 01:27

rebase

Harbormaster completed remote builds in B11438: Diff 21313.Jun 12 2020, 01:40
Fabien requested changes to this revision.Jun 12 2020, 07:22
Fabien added inline comments.
src/txmempool.cpp
1336 ↗(On Diff #21313)

I don't think this is correct.
There are only 3 callsites (tests excepted):

  • In DisconnectTip(), which this diff makes EXCLUSIVE_LOCKS_REQUIRED(::mempool.cs)
  • in removeForBlock(), which in turn is used in ConnectTip() which this diff males EXCLUSIVE_LOCKS_REQUIRED(::mempool.cs)
  • in addForBlock(), which in turn is used in DisconnectTip(), which this diff makes EXCLUSIVE_LOCKS_REQUIRED(::mempool.cs)

So if you need the lock here then either I am missing a callsite or there is a problem somewhere else that cause the lock not the be held as expected.

This revision now requires changes to proceed.Jun 12 2020, 07:22
majcosta updated this revision to Diff 21567.Jun 19 2020, 17:57

made addToBlock require exclusive lock on ::g_mempool and extended lock to callsite

majcosta edited the test plan for this revision. (Show Details)Jun 19 2020, 17:59
Harbormaster completed remote builds in B11590: Diff 21567.Jun 19 2020, 18:08
deadalnix added a subscriber: deadalnix.Jun 20 2020, 00:13

The title/description do not match what is actually backported.

deadalnix requested changes to this revision.Jun 20 2020, 00:14
This revision now requires changes to proceed.Jun 20 2020, 00:14
majcosta updated this revision to Diff 22040.EditedJul 7 2020, 00:34

void UpdateMempoolForReorg void UpdateMempoolForReorg and a section of bool DisconnectTip were refactored into DisconnectedBlockTransactions::updateMempoolForReorg and DisconnectedBlockTransactions::addForBlock in D1667

the former had a lock requirement annotation for CTxMemPool.cs, the latter did not, even though it calls CTxMemPool::removeRecursive which accesses CTxMemPool::mapTx which is guarded by the mutex

made sure the call to removeRecursive is preceded by locking it

Harbormaster completed remote builds in B11875: Diff 22040.Jul 7 2020, 00:58
majcosta planned changes to this revision.Jul 7 2020, 07:23
majcosta requested review of this revision.Jul 7 2020, 16:03
deadalnix requested changes to this revision.Jul 7 2020, 22:24

There is a missing lock when invalidating a block - and presumably parking too.

src/txmempool.cpp
1387 ↗(On Diff #22040)

This is not useful because it is already stated at the declaration location. An assertion on the lock is.

src/txmempool.h
57 ↗(On Diff #22040)

It looks like you are missing a dependency.

976 ↗(On Diff #22040)

Passing the mempool in explicitly would be preferable.

This revision now requires changes to proceed.Jul 7 2020, 22:24
majcosta mentioned this in D7439: [refactor] pass CTxMempool& to DisconnectedBlockTransactions methods.Sep 14 2020, 21:50
majcosta updated this revision to Diff 23482.Sep 14 2020, 22:00

rebase on D7439

majcosta added a child revision: D7440: [backport#14193][test] Add test to check mempool consistency in case of reorgs.Sep 14 2020, 22:09
Harbormaster completed remote builds in B12726: Diff 23482.Sep 14 2020, 22:14
deadalnix accepted this revision.Sep 15 2020, 22:30
majcosta added a comment.Sep 16 2020, 12:10

@bot build-tsan

teamcity added a comment.Sep 16 2020, 12:16
Build Bitcoin ABC Diffs / Diff Testing (build-tsan) failed.

Snippet of first build failure:

../../src/test/mempool_tests.cpp(325): Entering test case "MempoolIndexingTest"
../../src/test/mempool_tests.cpp(325): Leaving test case "MempoolIndexingTest"; testing time: 124211us
../../src/test/mempool_tests.cpp(519): Entering test case "MempoolAncestorIndexingTest"
../../src/test/mempool_tests.cpp(519): Leaving test case "MempoolAncestorIndexingTest"; testing time: 130342us
../../src/test/mempool_tests.cpp(661): Entering test case "MempoolSizeLimitTest"
../../src/test/mempool_tests.cpp(661): Leaving test case "MempoolSizeLimitTest"; testing time: 122904us
../../src/test/mempool_tests.cpp(842): Entering test case "TestImportMempool"
==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=5601)
  Cycle in lock order graph: M262865 (0x556da57e2010) => M1574750 (0x7ffc33fbeb68) => M262865

  Mutex M1574750 acquired here while holding mutex M262865 in main thread:
    #0 pthread_mutex_lock <null> (test_bitcoin+0xea1ee)
    #1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8/bits/gthr-default.h:748:12 (test_bitcoin+0x466220)
    #2 __gthread_recursive_mutex_lock(pthread_mutex_t*) /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8/bits/gthr-default.h:810 (test_bitcoin+0x466220)
    #3 std::recursive_mutex::lock() /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/mutex:107 (test_bitcoin+0x466220)
    #4 std::unique_lock<std::recursive_mutex>::lock() /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/std_mutex.h:267 (test_bitcoin+0x466220)
    #5 UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::Enter(char const*, char const*, int) /work/abc-ci-builds/build-tsan/../../src/./sync.h:129 (test_bitcoin+0x466220)
    #6 UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::UniqueLock(AnnotatedMixin<std::recursive_mutex>&, char const*, char const*, int, bool) /work/abc-ci-builds/build-tsan/../../src/./sync.h:151 (test_bitcoin+0x466220)
    #7 mempool_tests::MempoolSizeLimitTest::test_method() /work/abc-ci-builds/build-tsan/../../src/test/mempool_tests.cpp:663 (test_bitcoin+0x466220)
    #8 mempool_tests::MempoolSizeLimitTest_invoker() /work/abc-ci-builds/build-tsan/../../src/test/mempool_tests.cpp:661:1 (test_bitcoin+0x465a93)
    #9 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /usr/include/boost/function/function_template.hpp:118:11 (test_bitcoin+0x19a609)
    #10 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (libboost_unit_test_framework.so.1.67.0+0x55dbd)
    #11 __libc_start_main <null> (libc.so.6+0x2409a)

    Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative warning message

  Mutex M262865 acquired here while holding mutex M1574750 in main thread:
    #0 pthread_mutex_lock <null> (test_bitcoin+0xea1ee)
    #1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8/bits/gthr-default.h:748:12 (test_bitcoin+0x472fdb)
    #2 __gthread_recursive_mutex_lock(pthread_mutex_t*) /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8/bits/gthr-default.h:810 (test_bitcoin+0x472fdb)
    #3 std::recursive_mutex::lock() /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/mutex:107 (test_bitcoin+0x472fdb)
    #4 std::unique_lock<std::recursive_mutex>::lock() /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/std_mutex.h:267 (test_bitcoin+0x472fdb)
    #5 UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::Enter(char const*, char const*, int) /work/abc-ci-builds/build-tsan/../../src/./sync.h:129 (test_bitcoin+0x472fdb)
    #6 UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::UniqueLock(AnnotatedMixin<std::recursive_mutex>&, char const*, char const*, int, bool) /work/abc-ci-builds/build-tsan/../../src/./sync.h:151 (test_bitcoin+0x472fdb)
    #7 mempool_tests::TestImportMempool::test_method() /work/abc-ci-builds/build-tsan/../../src/test/mempool_tests.cpp:899 (test_bitcoin+0x472fdb)
    #8 mempool_tests::TestImportMempool_invoker() /work/abc-ci-builds/build-tsan/../../src/test/mempool_tests.cpp:842:1 (test_bitcoin+0x4717d3)
    #9 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /usr/include/boost/function/function_template.hpp:118:11 (test_bitcoin+0x19a609)
    #10 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (libboost_unit_test_framework.so.1.67.0+0x55dbd)
    #11 __libc_start_main <null> (libc.so.6+0x2409a)

SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) (/work/abc-ci-builds/build-tsan/src/test/test_bitcoin+0xea1ee) in pthread_mutex_lock
==================
../../src/test/mempool_tests.cpp(842): Leaving test case "TestImportMempool"; testing time: 161171us
../../src/test/mempool_tests.cpp(939): Entering test case "MempoolAncestryTests"
../../src/test/mempool_tests.cpp(939): Leaving test case "MempoolAncestryTests"; testing time: 133198us
../../src/test/mempool_tests.cpp(19): Leaving test suite "mempool_tests"; testing time: 1430136us
Leaving test module "Bitcoin ABC unit tests"; testing time: 1430476us

*** No errors detected
ThreadSanitizer: reported 1 warnings
[375/403] Running pow test suite
PASSED: pow test suite
[392/403] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[400/403] Running utility command for check-bitcoin-coinselector_tests
FAILED: test/CMakeFiles/check-functional 
cd /work/abc-ci-builds/build-tsan/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-tsan/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-tsan/test/log && /usr/bin/cmake -E env TSAN_OPTIONS=suppressions=/work/test/sanitizer_suppressions/tsan:log_path=stdout /usr/bin/python3.7 ./functional/test_runner.py "--testsuitename=Bitcoin ABC functional tests" --junitoutput=/work/abc-ci-builds/build-tsan/test/junit/functional_tests.xml
ninja: build stopped: cannot make progress due to previous errors.
Build build-tsan failed with exit code 1

Each failure log is accessible here:
Bitcoin ABC functional tests: interface_rest.py
Bitcoin ABC functional tests: interface_zmq.py
Bitcoin ABC functional tests: mempool_reorg.py
Bitcoin ABC functional tests: p2p_compactblocks.py
Bitcoin ABC functional tests: rpc_psbt.py
Bitcoin ABC functional tests: rpc_rawtransaction.py
Bitcoin ABC functional tests: wallet_abandonconflict.py
Bitcoin ABC functional tests: wallet_address_types.py
Bitcoin ABC functional tests: wallet_avoidreuse.py
Bitcoin ABC functional tests: wallet_backup.py
Bitcoin ABC functional tests: wallet_balance.py
Bitcoin ABC functional tests: wallet_basic.py
Bitcoin ABC functional tests: wallet_groups.py
Bitcoin ABC functional tests: wallet_listreceivedby.py
Bitcoin ABC functional tests: wallet_listtransactions.py
Bitcoin ABC functional tests: wallet_txn_clone.py --mineblock
Bitcoin ABC functional tests: wallet_txn_doublespend.py --mineblock

majcosta planned changes to this revision.Sep 16 2020, 14:10
majcosta updated this revision to Diff 23586.Sep 18 2020, 15:47

switch lock order on mempool_tests pointed by tsan

Harbormaster completed remote builds in B12798: Diff 23586.Sep 18 2020, 16:05
majcosta added a comment.Sep 18 2020, 16:09

@bot build-tsan

teamcity added a comment.Sep 18 2020, 16:19
Build Bitcoin ABC Diffs / Diff Testing (build-tsan) failed.

Snippet of first build failure:

wallet_hd.py                            | ✓ Passed  | 9 s
wallet_import_rescan.py                 | ✓ Passed  | 11 s
wallet_import_with_label.py             | ✓ Passed  | 2 s
wallet_importmulti.py                   | ✓ Passed  | 9 s
wallet_importprunedfunds.py             | ✓ Passed  | 3 s
wallet_keypool.py                       | ✓ Passed  | 4 s
wallet_keypool_topup.py                 | ✓ Passed  | 6 s
wallet_labels.py                        | ✓ Passed  | 4 s
wallet_listsinceblock.py                | ✓ Passed  | 7 s
wallet_multiwallet.py                   | ✓ Passed  | 60 s
wallet_multiwallet.py --usecli          | ✓ Passed  | 64 s
wallet_reorgsrestore.py                 | ✓ Passed  | 8 s
wallet_resendwallettransactions.py      | ✓ Passed  | 6 s
wallet_txn_clone.py --mineblock         | ✓ Passed  | 5 s
wallet_txn_doublespend.py               | ✓ Passed  | 3 s
wallet_watchonly.py                     | ✓ Passed  | 2 s
wallet_watchonly.py --usecli            | ✓ Passed  | 3 s
wallet_zapwallettxes.py                 | ✓ Passed  | 9 s
interface_rest.py                       | ✖ Failed  | 17 s
interface_zmq.py                        | ✖ Failed  | 28 s
mempool_reorg.py                        | ✖ Failed  | 5 s
p2p_compactblocks.py                    | ✖ Failed  | 15 s
p2p_feefilter.py                        | ✖ Failed  | 58 s
rpc_psbt.py                             | ✖ Failed  | 51 s
wallet_abandonconflict.py               | ✖ Failed  | 6 s
wallet_address_types.py                 | ✖ Failed  | 18 s
wallet_avoidreuse.py                    | ✖ Failed  | 10 s
wallet_balance.py                       | ✖ Failed  | 15 s
wallet_basic.py                         | ✖ Failed  | 68 s
wallet_groups.py                        | ✖ Failed  | 26 s
wallet_listreceivedby.py                | ✖ Failed  | 24 s
wallet_listtransactions.py              | ✖ Failed  | 19 s
wallet_txn_clone.py                     | ✖ Failed  | 4 s
wallet_txn_doublespend.py --mineblock   | ✖ Failed  | 5 s

ALL                                     | ✖ Failed  | 1446 s (accumulated) 
Runtime: 290 s

[165/403] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.005s

OK
[166/403] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[375/403] Running pow test suite
PASSED: pow test suite
[391/403] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[402/403] Running bitcoin test suite
PASSED: bitcoin test suite
FAILED: test/CMakeFiles/check-functional 
cd /work/abc-ci-builds/build-tsan/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-tsan/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-tsan/test/log && /usr/bin/cmake -E env TSAN_OPTIONS=suppressions=/work/test/sanitizer_suppressions/tsan:log_path=stdout /usr/bin/python3.7 ./functional/test_runner.py "--testsuitename=Bitcoin ABC functional tests" --junitoutput=/work/abc-ci-builds/build-tsan/test/junit/functional_tests.xml
ninja: build stopped: cannot make progress due to previous errors.
Build build-tsan failed with exit code 1

Each failure log is accessible here:
Bitcoin ABC functional tests: interface_rest.py
Bitcoin ABC functional tests: interface_zmq.py
Bitcoin ABC functional tests: mempool_reorg.py
Bitcoin ABC functional tests: p2p_compactblocks.py
Bitcoin ABC functional tests: p2p_feefilter.py
Bitcoin ABC functional tests: rpc_psbt.py
Bitcoin ABC functional tests: wallet_abandonconflict.py
Bitcoin ABC functional tests: wallet_address_types.py
Bitcoin ABC functional tests: wallet_avoidreuse.py
Bitcoin ABC functional tests: wallet_balance.py
Bitcoin ABC functional tests: wallet_basic.py
Bitcoin ABC functional tests: wallet_groups.py
Bitcoin ABC functional tests: wallet_listreceivedby.py
Bitcoin ABC functional tests: wallet_listtransactions.py
Bitcoin ABC functional tests: wallet_txn_clone.py
Bitcoin ABC functional tests: wallet_txn_doublespend.py --mineblock

majcosta planned changes to this revision.Sep 22 2020, 16:34
majcosta mentioned this in rABCbcc81a569012: [refactor] pass CTxMempool& to DisconnectedBlockTransactions methods.Sep 25 2020, 17:28
majcosta updated this revision to Diff 23805.Sep 25 2020, 17:34

rebase and fix lock-order inversion tsan warning on mempool_tests.cpp

Harbormaster completed remote builds in B12927: Diff 23805.Sep 25 2020, 18:03
majcosta added a comment.Sep 25 2020, 18:28

@bot build-tsan

Fabien accepted this revision.Sep 25 2020, 20:02
This revision is now accepted and ready to land.Sep 25 2020, 20:02
majcosta updated this revision to Diff 23821.Sep 25 2020, 23:21

revert mempool_tests.cpp to the state of the revision it was accepted

This revision was landed with ongoing or failed builds.Sep 25 2020, 23:31
Closed by commit rABC472a67420336: [backport#14193] validation: Add missing mempool locks (authored by MarcoFalke <falke.marco@gmail.com>, committed by majcosta). · Explain Why
This revision was automatically updated to reflect the committed changes.
majcosta added a commit: rABC472a67420336: [backport#14193] validation: Add missing mempool locks.
Harbormaster completed remote builds in B12935: Diff 23821.Sep 25 2020, 23:35

Revision Contents
Changeset List

PathSize
src/
rpc/
2 lines
test/
26 lines
39 lines
27 lines
7 lines
11 lines

Diff 23805

View Options

src/rpc/mining.cpp

Loading...
View Options

src/test/mempool_tests.cpp

Loading...
View Options

src/txmempool.h

Loading...
View Options

src/txmempool.cpp

Loading...
View Options

src/validation.h

Loading...
View Options

src/validation.cpp

Loading...