miner: Avoid stack-use-after-return in validationinterface
ClosedPublic
Actions

Authored by PiRK on Jan 28 2021, 14:32.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABCb3681986e349: miner: Avoid stack-use-after-return in validationinterface

Summary

This is a squash of 2 commits, to add the unbroken test and the solution
at the same time, and a revert of D5140 (different solution to the same problem)

test: Add unregister_validation_interface_race test

https://github.com/bitcoin/bitcoin/pull/18742/commits/fab6d060ce5f580db538070beec1c5518c8c777c

miner: Avoid stack-use-after-return in validationinterface

This is achieved by switching to a shared_ptr.

Also, switch the validationinterfaces in the tests to use shared_ptrs
for the same reason.

https://github.com/bitcoin/bitcoin/pull/18742/commits/7777f2a4bb1f9d843bc50a4e35085cfbb2808780

This is a backport of Core PR18742 [3/3]

This replaces the solution in D5140
See comment https://reviews.bitcoinabc.org/D5140#124338

Depends on D9104

Test Plan

Test suggested by this PR:

export ASAN_OPTIONS=detect_leaks=0
ninja && ninja check
while ./src/test/test_bitcoin -t validationinterface_tests/unregister_validation_interface_race --catch_system_errors=no  ; do true; done

Test from D5140:

CC=clang CXX=clang++ cmake -GNinja .. -DCMAKE_BUILD_TYPE=Debug
ninja check-extended

Diff Detail

Repository

rABC Bitcoin ABC

Branch

pr18742

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 15048
Build 30046: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 30045: arc lint + arc unit

Event Timeline

PiRK created this revision.Jan 28 2021, 14:32

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 28 2021, 14:32

PiRK requested review of this revision.Jan 28 2021, 14:32

teamcity edited the summary of this revision. (Show Details)Jan 28 2021, 14:32

PiRK edited the summary of this revision. (Show Details)Jan 28 2021, 14:33

PiRK added a parent revision: D9104: test: Remove UninterruptibleSleep from test and replace it by SyncWithValidationInterfaceQueue.

PiRK edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B15044: Diff 27411.Jan 28 2021, 14:57

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

Tail of the build log:

[0m[0;32mwallet_create_tx.py                              | ✓ Passed  | 6 s
[0m[0;32mwallet_createwallet.py                           | ✓ Passed  | 3 s
[0m[0;32mwallet_createwallet.py --usecli                  | ✓ Passed  | 3 s
[0m[0;32mwallet_descriptor.py                             | ✓ Passed  | 9 s
[0m[0;32mwallet_disable.py                                | ✓ Passed  | 1 s
[0m[0;32mwallet_dump.py                                   | ✓ Passed  | 5 s
[0m[0;32mwallet_encryption.py                             | ✓ Passed  | 5 s
[0m[0;32mwallet_groups.py                                 | ✓ Passed  | 46 s
[0m[0;32mwallet_hd.py                                     | ✓ Passed  | 5 s
[0m[0;32mwallet_import_rescan.py                          | ✓ Passed  | 10 s
[0m[0;32mwallet_import_with_label.py                      | ✓ Passed  | 1 s
[0m[0;32mwallet_importdescriptors.py                      | ✓ Passed  | 5 s
[0m[0;32mwallet_importmulti.py                            | ✓ Passed  | 5 s
[0m[0;32mwallet_importprunedfunds.py                      | ✓ Passed  | 1 s
[0m[0;32mwallet_keypool.py                                | ✓ Passed  | 3 s
[0m[0;32mwallet_keypool_topup.py                          | ✓ Passed  | 3 s
[0m[0;32mwallet_labels.py                                 | ✓ Passed  | 2 s
[0m[0;32mwallet_listreceivedby.py                         | ✓ Passed  | 14 s
[0m[0;32mwallet_listsinceblock.py                         | ✓ Passed  | 6 s
[0m[0;32mwallet_listtransactions.py                       | ✓ Passed  | 14 s
[0m[0;32mwallet_multiwallet.py                            | ✓ Passed  | 12 s
[0m[0;32mwallet_multiwallet.py --usecli                   | ✓ Passed  | 13 s
[0m[0;32mwallet_reorgsrestore.py                          | ✓ Passed  | 3 s
[0m[0;32mwallet_resendwallettransactions.py               | ✓ Passed  | 7 s
[0m[0;32mwallet_txn_clone.py                              | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_doublespend.py                        | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_doublespend.py --mineblock            | ✓ Passed  | 3 s
[0m[0;32mwallet_watchonly.py                              | ✓ Passed  | 1 s
[0m[0;32mwallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
[0m[0;32mwallet_zapwallettxes.py                          | ✓ Passed  | 5 s
[0m[1m
ALL                                              | ✓ Passed  | 1241 s (accumulated) 
[0mRuntime: 250 s

----------------------------------------------------------------------
Ran 1 test in 0.000s

OK

[179/438] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.014s

OK
[180/438] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[190/438] Running seeder test suite
PASSED: seeder test suite
[199/438] Running pow test suite
PASSED: pow test suite
[205/438] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1

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

Tail of the build log:

[0m[1;30mwallet_import_rescan.py                          | ○ Skipped | 0 s
[0m[1;30mwallet_import_with_label.py                      | ○ Skipped | 0 s
[0m[1;30mwallet_importdescriptors.py                      | ○ Skipped | 0 s
[0m[1;30mwallet_importmulti.py                            | ○ Skipped | 0 s
[0m[1;30mwallet_importprunedfunds.py                      | ○ Skipped | 0 s
[0m[1;30mwallet_keypool.py                                | ○ Skipped | 0 s
[0m[1;30mwallet_keypool_topup.py                          | ○ Skipped | 0 s
[0m[1;30mwallet_labels.py                                 | ○ Skipped | 0 s
[0m[1;30mwallet_listreceivedby.py                         | ○ Skipped | 0 s
[0m[1;30mwallet_listsinceblock.py                         | ○ Skipped | 0 s
[0m[1;30mwallet_listtransactions.py                       | ○ Skipped | 0 s
[0m[1;30mwallet_multiwallet.py                            | ○ Skipped | 0 s
[0m[1;30mwallet_multiwallet.py --usecli                   | ○ Skipped | 0 s
[0m[1;30mwallet_reorgsrestore.py                          | ○ Skipped | 0 s
[0m[1;30mwallet_resendwallettransactions.py               | ○ Skipped | 0 s
[0m[1;30mwallet_txn_clone.py                              | ○ Skipped | 0 s
[0m[1;30mwallet_txn_clone.py --mineblock                  | ○ Skipped | 0 s
[0m[1;30mwallet_txn_doublespend.py                        | ○ Skipped | 0 s
[0m[1;30mwallet_txn_doublespend.py --mineblock            | ○ Skipped | 0 s
[0m[1;30mwallet_watchonly.py                              | ○ Skipped | 0 s
[0m[1;30mwallet_watchonly.py --usecli                     | ○ Skipped | 0 s
[0m[1;30mwallet_zapwallettxes.py                          | ○ Skipped | 0 s
[0m[1m
ALL                                              | ✓ Passed  | 445 s (accumulated) 
[0mRuntime: 103 s

----------------------------------------------------------------------
Ran 1 test in 0.000s

OK

[145/401] Running avalanche test suite
PASSED: avalanche test suite
[147/401] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.005s

OK
[149/401] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[164/401] Running pow test suite
PASSED: pow test suite
[174/401] Running seeder test suite
PASSED: seeder test suite
[182/401] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[184/401] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/script_tests.cpp.o
In file included from /usr/include/boost/test/unit_test.hpp:19,
                 from ../../src/test/script_tests.cpp:30:
../../src/test/script_tests.cpp: In member function ‘void script_tests::script_build::test_method()’:
../../src/test/script_tests.cpp:541:22: note: variable tracking size limit exceeded with -fvar-tracking-assignments, retrying without
 BOOST_AUTO_TEST_CASE(script_build) {
                      ^~~~~~~~~~~~
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1

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

Tail of the build log:

[0m[0;32mrpc_psbt.py                                      | ✓ Passed  | 28 s
[0m[0;32mrpc_rawtransaction.py                            | ✓ Passed  | 37 s
[0m[0;32mrpc_scantxoutset.py                              | ✓ Passed  | 3 s
[0m[0;32mrpc_setban.py                                    | ✓ Passed  | 2 s
[0m[0;32mrpc_signmessage.py                               | ✓ Passed  | 1 s
[0m[0;32mrpc_signrawtransaction.py                        | ✓ Passed  | 1 s
[0m[0;32mrpc_txoutproof.py                                | ✓ Passed  | 2 s
[0m[0;32mrpc_uptime.py                                    | ✓ Passed  | 1 s
[0m[0;32mrpc_users.py                                     | ✓ Passed  | 5 s
[0m[0;32mrpc_whitelist.py                                 | ✓ Passed  | 1 s
[0m[0;32mtool_wallet.py                                   | ✓ Passed  | 4 s
[0m[0;32mwallet_abandonconflict.py                        | ✓ Passed  | 16 s
[0m[0;32mwallet_address_types.py                          | ✓ Passed  | 12 s
[0m[0;32mwallet_avoidreuse.py                             | ✓ Passed  | 4 s
[0m[0;32mwallet_backup.py                                 | ✓ Passed  | 23 s
[0m[0;32mwallet_balance.py                                | ✓ Passed  | 16 s
[0m[0;32mwallet_basic.py                                  | ✓ Passed  | 25 s
[0m[0;32mwallet_coinbase_category.py                      | ✓ Passed  | 1 s
[0m[0;32mwallet_create_tx.py                              | ✓ Passed  | 5 s
[0m[0;32mwallet_createwallet.py                           | ✓ Passed  | 2 s
[0m[0;32mwallet_createwallet.py --usecli                  | ✓ Passed  | 3 s
[0m[0;32mwallet_descriptor.py                             | ✓ Passed  | 6 s
[0m[0;32mwallet_disable.py                                | ✓ Passed  | 0 s
[0m[0;32mwallet_dump.py                                   | ✓ Passed  | 5 s
[0m[0;32mwallet_encryption.py                             | ✓ Passed  | 5 s
[0m[0;32mwallet_groups.py                                 | ✓ Passed  | 31 s
[0m[0;32mwallet_hd.py                                     | ✓ Passed  | 5 s
[0m[0;32mwallet_import_rescan.py                          | ✓ Passed  | 5 s
[0m[0;32mwallet_import_with_label.py                      | ✓ Passed  | 1 s
[0m[0;32mwallet_importdescriptors.py                      | ✓ Passed  | 4 s
[0m[0;32mwallet_importmulti.py                            | ✓ Passed  | 3 s
[0m[0;32mwallet_importprunedfunds.py                      | ✓ Passed  | 1 s
[0m[0;32mwallet_keypool.py                                | ✓ Passed  | 3 s
[0m[0;32mwallet_keypool_topup.py                          | ✓ Passed  | 2 s
[0m[0;32mwallet_labels.py                                 | ✓ Passed  | 1 s
[0m[0;32mwallet_listreceivedby.py                         | ✓ Passed  | 21 s
[0m[0;32mwallet_listsinceblock.py                         | ✓ Passed  | 3 s
[0m[0;32mwallet_listtransactions.py                       | ✓ Passed  | 23 s
[0m[0;32mwallet_multiwallet.py                            | ✓ Passed  | 12 s
[0m[0;32mwallet_multiwallet.py --usecli                   | ✓ Passed  | 12 s
[0m[0;32mwallet_reorgsrestore.py                          | ✓ Passed  | 3 s
[0m[0;32mwallet_resendwallettransactions.py               | ✓ Passed  | 10 s
[0m[0;32mwallet_txn_clone.py                              | ✓ Passed  | 1 s
[0m[0;32mwallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_doublespend.py                        | ✓ Passed  | 1 s
[0m[0;32mwallet_txn_doublespend.py --mineblock            | ✓ Passed  | 3 s
[0m[0;32mwallet_watchonly.py                              | ✓ Passed  | 1 s
[0m[0;32mwallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
[0m[0;32mwallet_zapwallettxes.py                          | ✓ Passed  | 4 s
[0m[1m
ALL                                              | ✓ Passed  | 973 s (accumulated) 
[0mRuntime: 195 s

----------------------------------------------------------------------
Ran 1 test in 0.000s

OK

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1