Page MenuHomePhabricator

Avoid locking mutexes that are already held by the same thread
ClosedPublic

Authored by fpelliccioni on Sep 24 2019, 12:35.

Details

Summary

Avoid locking mutexes that are already held by the same thread. These are reentrant mutexes, but still no need to lock them more than once per thread :-)

Backport of Bitcoin Core PR11762
https://github.com/bitcoin/bitcoin/pull/11762

Test Plan
  1. Build with Clang in Debug mode:
CXX=clang++ CC=clang cmake .. -GNinja -DCMAKE_BUILD_TYPE=Debug
ninja check
  1. Verify that the compiler has not emitted a thread-safety warning.
  2. Run the node: ./src/bitcoind -regtest
  3. Verify that text similar to "Assertion failed: lock ... not held ..." is not printed on stderr.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
arcpatch-D4145
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 7608
Build 13256: Bitcoin ABC Buildbot (legacy)
Build 13255: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Sep 24 2019, 13:49
deadalnix requested changes to this revision.Sep 24 2019, 13:50

The test plan is not apropriate. You need to test with clang to make sure the thread anotation are repsected, (gcc doesn't support this) and run the test suite on a debug build to check that AssertLockHeld doesn't trigger.

This revision now requires changes to proceed.Sep 24 2019, 13:50
fpelliccioni edited the test plan for this revision. (Show Details)
fpelliccioni edited the test plan for this revision. (Show Details)

Test Plan fixed.

This revision is now accepted and ready to land.Sep 26 2019, 10:20