Paths

Table of Contentst

Differential D9230

Avoid crash when g_thread_http was never started
ClosedPublic
Actions

Authored by PiRK on Feb 16 2021, 07:38.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABCeb6a052baa17: Avoid crash when g_thread_http was never started

Summary

Avoid a crash during shutdown when the init sequence failed for some reason

fa83b39ff3ae3fbad93df002915c0e5f99c104a9

init: Remove confusing and redundant InitError

The "A fatal internal error occurred, see debug.log for details" is
redundant because init.cpp will already show an InitError with a better
error message as well as the hint to check the debug.log

fa12a37b27f0570a551b8c103ea6537ee4a8e399

test: Replace inline-comments with logs, pep8 formatting

faf45d1f1f997c316fc4c611a23c4456533eefe9

http: Avoid crash when g_thread_http was never started

g_thread_http can not be joined when it is not joinable. Avoid crashing
the node by adding the required check and add a test.

This is a backport of Core PR19006

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Branch

pr19006

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 15210
Build 30358: Build Diff	build-diff · build-clang-tidy · build-debug · build-without-wallet · lint-circular-dependencies · build-clang
Build 30357: arc lint + arc unit

Event Timeline

PiRK created this revision.Feb 16 2021, 07:38

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 16 2021, 07:38

PiRK requested review of this revision.Feb 16 2021, 07:38

teamcity edited the summary of this revision. (Show Details)Feb 16 2021, 07:39

Harbormaster completed remote builds in B15205: Diff 27704.Feb 16 2021, 07:44

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

Tail of the build log:

[324/501] Building CXX object src/CMakeFiles/util.dir/blockdb.cpp.o
[325/501] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/uint256.cpp.o
[326/501] Building CXX object src/CMakeFiles/util.dir/util/error.cpp.o
[327/501] Building CXX object src/CMakeFiles/util.dir/util/settings.cpp.o
[328/501] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/util/strencodings.cpp.o
[329/501] Building CXX object src/CMakeFiles/util.dir/util/message.cpp.o
[330/501] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/primitives/transaction.cpp.o
[331/501] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/pubkey.cpp.o
[332/501] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/consensus/tx_check.cpp.o
[333/501] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[334/501] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[335/501] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[336/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[337/501] Building CXX object src/CMakeFiles/util.dir/util/time.cpp.o
[338/501] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[339/501] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[340/501] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[341/501] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[342/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[343/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[344/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[345/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[346/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[347/501] Building CXX object src/CMakeFiles/util.dir/util/system.cpp.o
[348/501] Linking CXX static library src/libutil.a
[349/501] Linking CXX static library src/librpcclient.a
[350/501] Linking CXX static library src/libbitcoinconsensus.a
[351/501] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[352/501] Linking CXX static library src/libscript.a
[353/501] Linking CXX static library src/libcommon.a
[354/501] Linking CXX shared library src/libbitcoinconsensus.so.0.22.13
[355/501] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[356/501] Linking CXX executable src/bitcoin-cli
[357/501] Linking CXX executable src/bitcoin-tx
[358/501] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqabstractnotifier.cpp.o
[359/501] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[360/501] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[361/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[362/501] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqnotificationinterface.cpp.o
[363/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[364/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[365/501] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[366/501] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqpublishnotifier.cpp.o
[367/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[368/501] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[369/501] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqrpc.cpp.o
[370/501] Linking CXX static library src/zmq/libzmq.a
[371/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[372/501] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[373/501] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[374/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[375/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[376/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[377/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[378/501] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[379/501] Linking CXX static library src/wallet/libwallet.a
[380/501] Linking CXX static library src/wallet/libwallet-tool.a
[381/501] Linking CXX executable src/bitcoin-wallet
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

Fabien requested changes to this revision.Feb 16 2021, 07:51

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/httprpc.cpp
10 ↗	(On Diff #27704)	Should it be removed like in the PR ?
src/httpserver.cpp
508 ↗	(On Diff #27704)	Braces

This revision now requires changes to proceed.Feb 16 2021, 07:51

PiRK added inline comments.Feb 16 2021, 09:25

src/httprpc.cpp
10 ↗	(On Diff #27704)	I missed this, thanks.

address review

Harbormaster completed remote builds in B15206: Diff 27705.Feb 16 2021, 09:34

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

Tail of the build log:

[378/433] Running utility command for check-bitcoin-sync_tests
[379/433] bitcoin: testing settings_tests
[380/433] Running utility command for check-bitcoin-torcontrol_tests
[381/433] Running utility command for check-bitcoin-settings_tests
[382/433] bitcoin: testing txvalidationcache_tests
[383/433] Running utility command for check-bitcoin-txvalidationcache_tests
[384/433] bitcoin: testing scriptpubkeyman_tests
[385/433] Running utility command for check-bitcoin-scriptpubkeyman_tests
[386/433] bitcoin: testing validation_block_tests
[387/433] Running utility command for check-bitcoin-validation_block_tests
[388/433] bitcoin: testing serialize_tests
[389/433] bitcoin: testing radix_tests
[390/433] Running utility command for check-bitcoin-serialize_tests
[391/433] Running utility command for check-bitcoin-radix_tests
[392/433] bitcoin: testing uint256_tests
[393/433] Running utility command for check-bitcoin-uint256_tests
[394/433] bitcoin: testing blockcheck_tests
[395/433] bitcoin: testing schnorr_tests
[396/433] bitcoin: testing walletdb_tests
[397/433] Running utility command for check-bitcoin-blockcheck_tests
[398/433] Running utility command for check-bitcoin-walletdb_tests
[399/433] bitcoin: testing undo_tests
[400/433] Running utility command for check-bitcoin-schnorr_tests
[401/433] Running utility command for check-bitcoin-undo_tests
[402/433] bitcoin: testing validation_chainstatemanager_tests
[403/433] bitcoin: testing ismine_tests
[404/433] Running utility command for check-bitcoin-validation_chainstatemanager_tests
[405/433] bitcoin: testing wallet_tests
[406/433] bitcoin: testing cashaddr_tests
[407/433] Running utility command for check-bitcoin-ismine_tests
[408/433] Running utility command for check-bitcoin-wallet_tests
[409/433] bitcoin: testing cuckoocache_tests
[410/433] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[411/433] Running utility command for check-bitcoin-cashaddr_tests
[412/433] Running utility command for check-bitcoin-cuckoocache_tests
[413/433] bitcoin: testing ref_tests
[414/433] bitcoin: testing script_tests
[415/433] bitcoin: testing crypto_tests
[416/433] Running utility command for check-bitcoin-ref_tests
[417/433] Running utility command for check-bitcoin-script_tests
[418/433] Running utility command for check-bitcoin-crypto_tests
[419/433] bitcoin: testing util_tests
[420/433] bitcoin: testing validation_tests
[421/433] bitcoin: testing coinselector_tests
FAILED: src/test/CMakeFiles/check-bitcoin-coinselector_tests 
cd /work/abc-ci-builds/build-clang-tidy/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-clang-tidy/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-clang-tidy/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-clang-tidy/test/log/bitcoin-coinselector_tests.log /work/abc-ci-builds/build-clang-tidy/src/test/test_bitcoin --run_test=coinselector_tests --logger=HRF,message:JUNIT,message,bitcoin-coinselector_tests.xml --catch_system_errors=no
Segmentation fault (core dumped)
[422/433] Running utility command for check-bitcoin-util_tests
[423/433] Running utility command for check-bitcoin-validation_tests
[424/433] bitcoin: testing skiplist_tests
[425/433] Running utility command for check-bitcoin-skiplist_tests
[426/433] bitcoin: testing op_reversebytes_tests
[427/433] Running utility command for check-bitcoin-op_reversebytes_tests
[428/433] bitcoin: testing transaction_tests
[429/433] Running utility command for check-bitcoin-transaction_tests
[430/433] bitcoin: testing coins_tests
[431/433] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

Fabien accepted this revision.Feb 16 2021, 10:05

This revision is now accepted and ready to land.Feb 16 2021, 10:05

I wasn't able to reproduce the latest build-clang-tidy failure on my machine.

@bot build-clang-tidy

rebase

Harbormaster completed remote builds in B15210: Diff 27710.Feb 16 2021, 16:39

Closed by commit rABCeb6a052baa17: Avoid crash when g_thread_http was never started (authored by MarcoFalke <falke.marco@gmail.com>, committed by PiRK). · Explain WhyFeb 17 2021, 07:38

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABCeb6a052baa17: Avoid crash when g_thread_http was never started.