Page MenuHomePhabricator

refactor: Avoid locking tx pool cs thrice
ClosedPublic

Authored by fpelliccioni on Tue, Oct 1, 13:32.

Details

Summary

addUnchecked is (outside the tests) only called by ATMP, which already takes the tx pool read lock. So locking it twice more in both addUnchecked methods seems redundant.

Backport of Bitcoin Core PR13786
https://github.com/bitcoin/bitcoin/pull/13786

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fpelliccioni created this revision.Tue, Oct 1, 13:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Oct 1, 13:32
Fabien requested changes to this revision.Tue, Oct 1, 14:11

Build with Clang to check for -Wthread-safety-warnings.

This revision now requires changes to proceed.Tue, Oct 1, 14:11
fpelliccioni edited the test plan for this revision. (Show Details)Wed, Oct 2, 16:51
fpelliccioni updated this revision to Diff 13325.Wed, Oct 2, 18:08

fixes building error.

deadalnix accepted this revision.Wed, Oct 2, 20:57
fpelliccioni updated this revision to Diff 13332.Thu, Oct 3, 11:52

fixes test plan.

Fabien accepted this revision.Thu, Oct 3, 13:26
This revision is now accepted and ready to land.Thu, Oct 3, 13:26
This revision was automatically updated to reflect the committed changes.