Merge #18551: Do not clear validationinterface entries being executed
ClosedPublic
Actions

Authored by jasonbcox on Jun 19 2020, 01:24.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC1064bda6981c: Merge #18551: Do not clear validationinterface entries being executed

Summary

2276339a176f83ffe8ceefb3e41ecca8601aa13b Add test for UnregisterAllValidationInterfaces bug (Russell Yanofsky)
3c61abbbc847d725f30d169278d84655571407c1 Do not clear validationinterface entries being executed (Pieter Wuille)

Pull request description:

The previous code for MainSignalsInstance::Clear would decrement the reference
count of every interface, including ones that were already Unregister()ed but
still being executed.

This fixes the issue pointed out here: https://github.com/bitcoin/bitcoin/pull/18524/files#r404395685 . It's not currently observable.

ACKs for top commit:

jonasschnelli:
  utACK 2276339a176f83ffe8ceefb3e41ecca8601aa13b - reviewed code and test (thanks @ryanofsky for adding the test).
MarcoFalke:
  ACK 2276339a176f83ffe8ceefb3e41ecca8601aa13b 🎎
ryanofsky:
  Code review ACK 2276339a176f83ffe8ceefb3e41ecca8601aa13b. No change to bugfix, just rebased and new test commit added since last review

Tree-SHA512: c1d68e7c681a45c6cadc84e407c2266bcb4b12d34264e1232a61c4eadb74b551231c5a3b1d041de39f507aef4dfa7d4589b8bfe1833f069c739c6270d2a05dbe

Backport of Core PR18551

Depends on D6653

Test Plan

ninja check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

pr18551

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 11585
Build 22449: Default Diff Build & Tests	Link to the build
Build 22448: Build without Wallet	Link to the build
Build 22447: Build with clang-10	Link to the build
Build 22446: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Jun 19 2020, 01:24

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 19 2020, 01:24

jasonbcox requested review of this revision.Jun 19 2020, 01:24

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

jasonbcox added a child revision: D6655: Merge #18563: test: Fix unregister_all_during_call cleanup.Jun 19 2020, 01:26

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Snippet of first build failure:

[01:28:11] :	 [Step 1/1] [94/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/sighash_tests.cpp.o
[01:28:11] :	 [Step 1/1] [95/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/sighashtype_tests.cpp.o
[01:28:11] :	 [Step 1/1] [96/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/sigcheckcount_tests.cpp.o
[01:28:11] :	 [Step 1/1] [97/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/skiplist_tests.cpp.o
[01:28:11] :	 [Step 1/1] [98/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/streams_tests.cpp.o
[01:28:11] :	 [Step 1/1] [99/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/sync_tests.cpp.o
[01:28:11] :	 [Step 1/1] [100/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/timedata_tests.cpp.o
[01:28:11] :	 [Step 1/1] [101/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/torcontrol_tests.cpp.o
[01:28:11] :	 [Step 1/1] [102/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/transaction_tests.cpp.o
[01:28:11] :	 [Step 1/1] [103/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/txvalidation_tests.cpp.o
[01:28:11] :	 [Step 1/1] [104/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/uint256_tests.cpp.o
[01:28:11] :	 [Step 1/1] [105/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/txvalidationcache_tests.cpp.o
[01:28:11] :	 [Step 1/1] [106/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/undo_tests.cpp.o
[01:28:11] :	 [Step 1/1] [107/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/util_threadnames_tests.cpp.o
[01:28:11] :	 [Step 1/1] [108/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/util_tests.cpp.o
[01:28:11] :	 [Step 1/1] [109/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/work_comparator_tests.cpp.o
[01:28:11] :	 [Step 1/1] [110/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/rpc/test/server_tests.cpp.o
[01:28:11] :	 [Step 1/1] [111/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/validation_tests.cpp.o
[01:28:11] :	 [Step 1/1] [112/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/versionbits_tests.cpp.o
[01:28:13] :	 [Step 1/1] [113/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/validationinterface_tests.cpp.o
[01:28:17] :	 [Step 1/1] [114/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/txindex_tests.cpp.o
[01:28:18] :	 [Step 1/1] [115/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/util/setup_common.cpp.o
[01:28:18] :	 [Step 1/1] [116/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/denialofservice_tests.cpp.o
[01:28:18] :	 [Step 1/1] [117/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/blockfilter_index_tests.cpp.o
[01:28:18] :	 [Step 1/1] [118/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/validation_block_tests.cpp.o
[01:28:20] :	 [Step 1/1] [119/120] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/avalanche_tests.cpp.o
[01:28:22] :	 [Step 1/1] [120/120] Linking CXX executable src/test/test_bitcoin
[01:28:22]W:	 [Step 1/1] + TEST_BITCOIN_JUNIT=junit_results_unit_tests_without_wallet.xml
[01:28:22]W:	 [Step 1/1] + TEST_BITCOIN_SUITE_NAME='Bitcoin ABC unit tests without wallet'
[01:28:22]W:	 [Step 1/1] + ASAN_OPTIONS=malloc_context_size=0:log_path=/tmp/sanitizer_logs/asan.log
[01:28:22]W:	 [Step 1/1] + LSAN_OPTIONS=suppressions=/work/test/sanitizer_suppressions/lsan:log_path=/tmp/sanitizer_logs/lsan.log
[01:28:22]W:	 [Step 1/1] + TSAN_OPTIONS=suppressions=/work/test/sanitizer_suppressions/tsan:log_path=/tmp/sanitizer_logs/tsan.log
[01:28:22]W:	 [Step 1/1] + UBSAN_OPTIONS=suppressions=/work/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:log_path=/tmp/sanitizer_logs/ubsan.log
[01:28:22]W:	 [Step 1/1] + ./src/test/test_bitcoin --logger=HRF:JUNIT,message,junit_results_unit_tests_without_wallet.xml -- '-testsuitename=Bitcoin ABC unit tests without wallet'
[01:28:58] :	 [Step 1/1] Running 440 test cases...
[01:28:58] :	 [Step 1/1] 
[01:28:58] :	 [Step 1/1] Assertion occurred in a following context:
[01:28:58] :	 [Step 1/1]     ../src/test/sigcheckcount_tests.cpp:335
[01:28:58] :	 [Step 1/1] Assertion occurred in a following context:
[01:28:58] :	 [Step 1/1]     ../src/test/sigcheckcount_tests.cpp:335
[01:28:58] :	 [Step 1/1] Assertion occurred in a following context:
[01:28:58] :	 [Step 1/1]     ../src/test/sigcheckcount_tests.cpp:339
[01:28:58] :	 [Step 1/1] Assertion occurred in a following context:
[01:28:58] :	 [Step 1/1]     ../src/test/sigcheckcount_tests.cpp:339
[01:28:58] :	 [Step 1/1] Assertion occurred in a following context:
[01:28:58] :	 [Step 1/1]     ../src/test/sigcheckcount_tests.cpp:349
[01:28:58] :	 [Step 1/1] Assertion occurred in a following context:
[01:28:58] :	 [Step 1/1]     ../src/test/sigcheckcount_tests.cpp:349
[01:28:58] :	 [Step 1/1] Assertion occurred in a following context:
[01:28:58] :	 [Step 1/1]     ../src/test/sigcheckcount_tests.cpp:359
[01:28:58] :	 [Step 1/1] Assertion occurred in a following context:
[01:29:06]W:	 [Step 1/1] test_bitcoin: ../src/validationinterface.cpp:97: void CMainSignals::RegisterBackgroundSignalScheduler(CScheduler&): Assertion `!m_internals' failed.
[01:29:06] :	 [Step 1/1]     ../src/test/sigcheckcount_tests.cpp:359unknown location(0):  [4;31;49mfatal error: in "versionbits_tests/versionbits_test": signal: SIGSEGV, si_code: -432025520 (memory access violation at address: 0x00000040) [0;39;49m
[01:29:06]W:	 [Step 1/1] 
[01:29:06] :	 [Step 1/1] ../src/test/versionbits_tests.cpp(226):  [1;36;49mlast checkpoint: "versionbits_test" fixture ctor [0;39;49m
[01:29:06]W:	 [Step 1/1]  [1;31;49m*** 1 failure is detected in the test module "Bitcoin ABC unit tests without wallet"
[01:29:06]W:	 [Step 1/1]  [0;39;49mcp: cannot stat '/work/build': No such file or directory
[01:29:06]W:	 [Step 1/1] cp: cannot stat '/work/ibd/debug.log': No such file or directory
[01:29:10]W:	 [Step 1/1] Process exited with code 201
[01:29:10]E:	 [Step 1/1] Process exited with code 201 (Step: Command Line)

Harbormaster completed remote builds in B11585: Diff 21561.Jun 19 2020, 01:29

Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Snippet of first build failure:

Build 'Bitcoin ABC Diffs / Diff Testing' #11895, branch 'phabricator/diff/21561'
Started 2020-06-19 01:25:33 on 'highperf1' by 'Phabricator Staging (phabricator-staging)'
Finished 2020-06-19 06:25:39 with status FAILURE 'Execution timeout (new)'

Fabien accepted this revision.Jun 19 2020, 07:15

Fabien added a subscriber: Fabien.

Fabien added inline comments.