Skip to content

Conversation

practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Nov 6, 2017

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).

@fanquake fanquake added the Tests label Nov 6, 2017
@promag
Copy link
Contributor

promag commented Nov 7, 2017

utACK 361136d.

@@ -66,11 +66,14 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
dummyNode1.fSuccessfullyConnected = true;
Copy link
Member

@laanwj laanwj Nov 8, 2017

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)

Copy link
Contributor Author

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 :-))

Copy link
Member

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...

@@ -81,7 +81,7 @@ CBlockIndex CreateBlockIndex(int nHeight)

bool TestSequenceLocks(const CTransaction &tx, int flags)
{
LOCK(mempool.cs);
LOCK2(cs_main, mempool.cs);
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Fixed!

@maflcko
Copy link
Member

maflcko commented Nov 10, 2017

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).
@maflcko
Copy link
Member

maflcko commented Nov 10, 2017

re-utACK 109a858

@maflcko maflcko merged commit 109a858 into bitcoin:master Nov 10, 2017
maflcko pushed a commit that referenced this pull request Nov 10, 2017
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
@promag
Copy link
Contributor

promag commented Nov 10, 2017

re-utACK 109a858.

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 27, 2019
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 2, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 4, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 16, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 22, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 22, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
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>
@practicalswift practicalswift deleted the test-locks branch April 10, 2021 19:33
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 30, 2021
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
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Feb 10, 2022
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants