-
Notifications
You must be signed in to change notification settings - Fork 37.7k
tests: Add missing locks to tests #11623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
utACK 361136d. |
@@ -66,11 +66,14 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) | |||
dummyNode1.fSuccessfullyConnected = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In tests where no multi-threading happens (most of them) it would work just as well to just take the necessary locks once in the beginning of each test case. That would make the diff smaller likely.
(but maybe explicit is better, I don't know)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@laanwj Do you know which tests in this PR that do not have any multi-threading? What is the best way to identify the tests for which multi-threading happens?
(Personally I like explicit locking better since that communicates where/why the lock is needed which is helpful for locking newcomers. I'll happily adjust to consensus however :-))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'd say just keep it like this...
src/test/miner_tests.cpp
Outdated
@@ -81,7 +81,7 @@ CBlockIndex CreateBlockIndex(int nHeight) | |||
|
|||
bool TestSequenceLocks(const CTransaction &tx, int flags) | |||
{ | |||
LOCK(mempool.cs); | |||
LOCK2(cs_main, mempool.cs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lock is already acquired in src/test/miner_tests.cpp:208, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Fixed!
utACK 361136d45e354e303fb96ce25c4b3945505633ab |
Add missing locks to tests to satisfy lock requirements (such as EXCLUSIVE_LOCKS_REQUIRED(...) (Clang Thread Safety Analysis), AssertLockHeld(...) and implicit lock assumptions).
361136d
to
109a858
Compare
re-utACK 109a858 |
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see #11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9
re-utACK 109a858. |
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org>
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
109a858 tests: Add missing locks to tests (practicalswift) Pull request description: Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see bitcoin#11226), `AssertLockHeld(...)` and implicit lock assumptions). Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/test/test_bitcoin.cpp
Add missing locks to tests to satisfy lock requirements (such as
EXCLUSIVE_LOCKS_REQUIRED(...)
(Clang Thread Safety Analysis, see #11226),AssertLockHeld(...)
and implicit lock assumptions).