e1e68b6305beb47ebf7ee48f14e12fdebdfea1ef test: Fix inconsistent lock order in wallet_tests/CreateWallet (Hennadii Stepanov)
cb23fe01c125e1820f3c37348e06d98c93e6aec2 sync: Check precondition in LEAVE_CRITICAL_SECTION() macro (Hennadii Stepanov)
c5e3e74f70c29ac8852903ef425f5f327d5da969 sync: Improve CheckLastCritical() (Hennadii Stepanov)
Pull request description:
This PR:
- fixes#19049 that was caused by #16426
- removes `wallet_tests::CreateWallet` suppression from the `test/sanitizer_suppressions/tsan`
The example of the improved `CheckLastCritical()`/`LEAVE_CRITICAL_SECTION()` log (could be got when compiled without the last commit):
```
2020-09-20T08:34:28.429485Z [test] INCONSISTENT LOCK ORDER DETECTED
2020-09-20T08:34:28.429493Z [test] Current lock order (least recent first) is:
2020-09-20T08:34:28.429501Z [test] 'walletInstance->cs_wallet' in wallet/wallet.cpp:4007 (in thread 'test')
2020-09-20T08:34:28.429508Z [test] 'cs_wallets' in wallet/wallet.cpp:4089 (in thread 'test')
```
Currently, there are other "naked" `LEAVE_CRITICAL_SECTION()` in the code base:
b99a1633b2/src/rpc/mining.cpp (L698)b99a1633b2/src/checkqueue.h (L208)
ACKs for top commit:
MarcoFalke:
review ACK e1e68b6305beb47ebf7ee48f14e12fdebdfea1ef 💂
ryanofsky:
Code review ACK e1e68b6305beb47ebf7ee48f14e12fdebdfea1ef. Just trivial rebase and suggested switch to BOOST_CHECK_EXCEPTION since last review
vasild:
ACK e1e68b630
Tree-SHA512: a627680eac3af4b4c02772473d68322ce8d3811bf6b035d3485ccc97d35755bef933cffabd3f20b126f89e3301eccecec3f769df34415fb7c426c967b6ce36e6
This backport does not include changes that depend on bitcoin pr 18037
70a6b529f306ff72ea1badf25e970a92b2b17ab3 lint-cppcheck: Remove -DHAVE_WORKING_BOOST_SLEEP_FOR (Anthony Towns)
294937b39de5924e772f8ed90d35c53290c8acab scheduler_tests: re-enable mockforward test (Anthony Towns)
cea19f685915be8affb2203184a549576194413f Drop unused reverselock.h (Anthony Towns)
d0ebd93270758ea97ea956b8821e17a2d001ea94 scheduler: switch from boost to std (Anthony Towns)
b9c426012770d166e6ebfab27689be44e6e89aa5 sync.h: add REVERSE_LOCK (Anthony Towns)
306f71b4eb4a0fd8e64f47dc008bc235b80b13d9 scheduler: don't rely on boost interrupt on shutdown (Anthony Towns)
Pull request description:
Replacing boost functionality with C++11 stuff.
Motivated by #18227, but should stand alone. Changing from `boost::condition_var` to `std::condition_var` means `threadGroup.interrupt_all` isn't enough to interrupt `serviceQueue` anymore, so that means calling `stop()` before `join_all()` is needed. And the existing reverselock.h code doesn't work with sync.h's DebugLock code (because the reversed lock won't be removed from `g_lockstack` which then leads to incorrect potential deadlock warnings), so I've replaced that with a dedicated class and macro that's aware of our debug lock behaviour.
Fixes#16027, Fixes#14200, Fixes#18227
ACKs for top commit:
laanwj:
ACK 70a6b529f306ff72ea1badf25e970a92b2b17ab3
Tree-SHA512: d1da13adeabcf9186d114e2dad9a4fdbe2e440f7afbccde0c13dfbaf464efcd850b69d3371c5bf8b179d7ceb9d81f4af3cc22960b90834e41eaaf6d52ef7d331
# Conflicts:
# src/reverselock.h
# src/rpc/misc.cpp
# src/scheduler.cpp
# src/scheduler.h
# src/sync.cpp
# src/sync.h
# src/test/reverselock_tests.cpp
# src/test/scheduler_tests.cpp
# src/test/test_dash.cpp
# test/lint/extended-lint-cppcheck.sh
* scripted-diff: Replace #include "" with #include <> (ryanofsky)
-BEGIN VERIFY SCRIPT-
for f in \
src/*.cpp \
src/*.h \
src/bench/*.cpp \
src/bench/*.h \
src/compat/*.cpp \
src/compat/*.h \
src/consensus/*.cpp \
src/consensus/*.h \
src/crypto/*.cpp \
src/crypto/*.h \
src/crypto/ctaes/*.h \
src/policy/*.cpp \
src/policy/*.h \
src/primitives/*.cpp \
src/primitives/*.h \
src/qt/*.cpp \
src/qt/*.h \
src/qt/test/*.cpp \
src/qt/test/*.h \
src/rpc/*.cpp \
src/rpc/*.h \
src/script/*.cpp \
src/script/*.h \
src/support/*.cpp \
src/support/*.h \
src/support/allocators/*.h \
src/test/*.cpp \
src/test/*.h \
src/wallet/*.cpp \
src/wallet/*.h \
src/wallet/test/*.cpp \
src/wallet/test/*.h \
src/zmq/*.cpp \
src/zmq/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Signed-off-by: Pasta <pasta@dashboost.org>
* scripted-diff: Replace #include "" with #include <> (Dash Specific)
-BEGIN VERIFY SCRIPT-
for f in \
src/bls/*.cpp \
src/bls/*.h \
src/evo/*.cpp \
src/evo/*.h \
src/governance/*.cpp \
src/governance/*.h \
src/llmq/*.cpp \
src/llmq/*.h \
src/masternode/*.cpp \
src/masternode/*.h \
src/privatesend/*.cpp \
src/privatesend/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Signed-off-by: Pasta <pasta@dashboost.org>
* build: Remove -I for everything but project root
Remove -I from build system for everything but the project root,
and built-in dependencies.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/Makefile.test.include
* qt: refactor: Use absolute include paths in .ui files
* qt: refactor: Changes to make include paths absolute
This makes all include paths in the GUI absolute.
Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/qt/dash.cpp
# src/qt/optionsmodel.cpp
# src/qt/test/rpcnestedtests.cpp
* test: refactor: Use absolute include paths for test data files
* Recommend #include<> syntax in developer notes
* refactor: Include obj/build.h instead of build.h
* END BACKPORT #11651 Remove trailing whitespace causing travis failure
* fix backport 11651
Signed-off-by: Pasta <pasta@dashboost.org>
* More of 11651
* fix blockchain.cpp
Signed-off-by: pasta <pasta@dashboost.org>
* Add missing "qt/" in includes
* Add missing "test/" in includes
* Fix trailing whitespaces
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
noexcept is default for destructors as of c++11. By throwing in reverselock's
destructor if it's lock has been tampered with, the likely result is
std::terminate being called. Indeed that happened before this change.
Once reverselock has taken another lock (its ctor didn't throw), it makes no
sense to try to grab or lock the parent lock. That is be broken/undefined
behavior depending on the parent lock's implementation, but it shouldn't cause
the reverselock to fail to re-lock when destroyed.
To avoid those problems, simply swap the parent lock's contents with a dummy
for the duration of the lock. That will ensure that any undefined behavior is
caught at the call-site rather than the reverse lock's destruction.
Barring a failed mutex unlock which would be indicative of a larger problem,
the destructor should now never throw.
