Skip to content

node: avoid recomputing block header hash in ReadBlock #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

node: avoid recomputing block header hash in ReadBlock #15

wants to merge 8 commits into from

Conversation

l0rinc
Copy link
Owner

@l0rinc l0rinc commented May 12, 2025

No description provided.

@l0rinc l0rinc changed the title node: avoid recomputing block hash in ReadBlock node: avoid recomputing block header hash in ReadBlock May 12, 2025
@l0rinc
Copy link
Owner Author

l0rinc commented May 13, 2025 

Benchmark 9: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=888888 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 -dbcache=4500 -swiftsyncfile=../swiftsync-888888.bin (COMMIT = 888e83d29ac9b8586c95acc40fd3ddd958e8ff4e)
  Time (abs ≡):        6449.086 s               [User: 7666.612 s, System: 523.872 s]

Benchmark 10: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=888888 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 -dbcache=4500 -swiftsyncfile=../swiftsync-888888.bin (COMMIT = 74999b15df4fa0c5126b4060a6129b56f769e869)
  Time (abs ≡):        6439.368 s               [User: 7646.494 s, System: 510.418 s]

MarcoFalke and others added 8 commits May 13, 2025 09:31
Remove HaveKey helper, unused after sethdseed removal
fa7b7f7
Remove unused LegacyDataSPKM::HaveWatchOnly()
fabdc5a
@hebasto
Squashed 'src/secp256k1/' changes from 0cdc758a56..4187a46649
c31fcaa 
4187a46649 Merge bitcoin-core/secp256k1#1492: tests: Add Wycheproof ECDH vectors
e266ba11ae tests: Add Wycheproof ECDH vectors
13906b7154 Merge bitcoin-core/secp256k1#1669: gitignore: Add Python cache files
c1bcb03276 gitignore: Add Python cache files
70f149b9a1 Merge bitcoin-core/secp256k1#1662: bench: add ellswift to bench help output
6b3fe51fb6 bench: add ellswift to bench help output
d84bb83e26 Merge bitcoin-core/secp256k1#1661: configure: Show exhaustive tests in summary
3f54ed8c1b Merge bitcoin-core/secp256k1#1659: include: remove WARN_UNUSED_RESULT for functions always returning 1
20b05c9d3f configure: Show exhaustive tests in summary
e56716a3bc Merge bitcoin-core/secp256k1#1660: ci: Fix exiting from ci.sh on error
d87c3bc58f ci: Fix exiting from ci.sh on error
1b6e081538 include: remove WARN_UNUSED_RESULT for functions always returning 1
2abb35b034 Merge bitcoin-core/secp256k1#1657: tests: remove unused uncounting_illegal_callback_fn
51907fa918 tests: remove unused uncounting_illegal_callback_fn
a7a5117144 Merge bitcoin-core/secp256k1#1359: Fix symbol visibility issues, add test for it
13ed6f65dc Merge bitcoin-core/secp256k1#1593: Remove deprecated `_ec_privkey_{negate,tweak_add,tweak_mul}` aliases from API
d1478763a5 build: Drop no longer needed  `-fvisibility=hidden` compiler option
8ed1d83d92 ci: Run `tools/symbol-check.py`
41d32ab2de test: Add `tools/symbol-check.py`
88548058b3 Introduce `SECP256K1_LOCAL_VAR` macro
03bbe8c615 Merge bitcoin-core/secp256k1#1655: gha: Print all *.log files, in a separate action
59860bcc24 gha: Print all *.log files, in a separate action
4ba1ba2af9 Merge bitcoin-core/secp256k1#1647: cmake: Adjust diagnostic flags for `clang-cl`
abd25054a1 Merge bitcoin-core/secp256k1#1656: musig: Fix clearing of pubnonces
961ec25a83 musig: Fix clearing of pubnonces
3186082387 Merge bitcoin-core/secp256k1#1614: Add _ge_set_all_gej and use it in musig for own public nonces
6c2a39dafb Merge bitcoin-core/secp256k1#1639: Make static context const
37d2c60bec Remove deprecated _ec_privkey_{negate,tweak_add,tweak_mul} aliases
432ac57705 Make static context const
1b1fc09341 Merge bitcoin-core/secp256k1#1642: Verify `compressed` argument in `secp256k1_eckey_pubkey_serialize`
c0d9480fbb Merge bitcoin-core/secp256k1#1654: use `EXIT_` constants over magic numbers for indicating program execution status
13d389629a CONTRIBUTING: mention that `EXIT_` codes should be used
c855581728 test, bench, precompute_ecmult: use `EXIT_...` constants for `main` return values
965393fcea examples: use `EXIT_...` constants for `main` return values
2e3bf13653 Merge bitcoin-core/secp256k1#1646: README: add instructions for verifying GPG signatures
b682dbcf84 README: add instructions for verifying GPG signatures
00774d0723 Merge bitcoin-core/secp256k1#1650: schnorrsig: clear out masked secret key in BIP-340 nonce function
a82287fb85 schnorrsig: clear out masked secret key in BIP-340 nonce function
4c50d73dd9 ci: Add new "Windows (clang-cl)" job
84c0bd1f72 cmake: Adjust diagnostic flags for clang-cl
f79f46c703 Merge bitcoin-core/secp256k1#1641: doc: Improve cmake instructions in README
2ac9f558c4 doc: Improve cmake instructions in README
1823594761 Verify `compressed` argument in `secp256k1_eckey_pubkey_serialize`
8deef00b33 Merge bitcoin-core/secp256k1#1634: Fix some misspellings
39705450eb Fix some misspellings
ec329c2501 Merge bitcoin-core/secp256k1#1633: release cleanup: bump version after 0.6.0
c97059f594 release cleanup: bump version after 0.6.0
64228a648f musig: Use _ge_set_all_gej for own public nonces
300aab1c05 tests: Improve _ge_set_all_gej(_var) tests
365f274ce3 group: Simplify secp256k1_ge_set_all_gej
d3082ddead group: Add constant-time secp256k1_ge_set_all_gej

git-subtree-dir: src/secp256k1
git-subtree-split: 4187a4664914dc6f6fb6a619c6b85c854fc33033
@hebasto
Update secp256k1 subtree to latest master
915c1fa
@fanquake
Merge bitcoin#32476: refactor: Remove unused HaveKey and HaveWatchOnly
cbd8e3d 
fabdc5a Remove unused LegacyDataSPKM::HaveWatchOnly() (MarcoFalke)
fa7b7f7 Remove HaveKey helper, unused after sethdseed removal (MarcoFalke)

Pull request description:

  removes some dead code

ACKs for top commit:
  Sjors:
    utACK fabdc5a

Tree-SHA512: 3cf6838eff6cedfe6a7337071d41e17d34033a2875b3661a6a1741e13c7a8e81b57eb874aebbb9d96d05ea7efa261a3a98e1bcf982b12cfafb163405d860b8bf
@fanquake
Merge bitcoin#32028: Update secp256k1 subtree to latest master
8309a97 
c31fcaa Squashed 'src/secp256k1/' changes from 0cdc758a56..4187a46649 (Hennadii Stepanov)

Pull request description:

  This PR updates the `secp256k1` subtree to bitcoin-core/secp256k1@4ba1ba2, which includes the following changes:
  - bitcoin-core/secp256k1#1633
  - bitcoin-core/secp256k1#1634
  - bitcoin-core/secp256k1#1641
  - bitcoin-core/secp256k1#1650
  - bitcoin-core/secp256k1#1646
  - bitcoin-core/secp256k1#1654
  - bitcoin-core/secp256k1#1642
  - bitcoin-core/secp256k1#1639
  - bitcoin-core/secp256k1#1614
  - bitcoin-core/secp256k1#1656
  - bitcoin-core/secp256k1#1647
  - bitcoin-core/secp256k1#1655
  - bitcoin-core/secp256k1#1593
  - bitcoin-core/secp256k1#1359
  - bitcoin-core/secp256k1#1657
  - bitcoin-core/secp256k1#1660
  - bitcoin-core/secp256k1#1659
  - bitcoin-core/secp256k1#1661
  - bitcoin-core/secp256k1#1662
  - bitcoin-core/secp256k1#1669
  - bitcoin-core/secp256k1#1492

  bitcoin-core/secp256k1#1647 is required for bitcoin#31507.

ACKs for top commit:
  fanquake:
    ACK 915c1fa

Tree-SHA512: 2c409ef2cd14fb14c2ea108f65411b673da786e886aabf75015be25220d5a3983ad8d9e76db1cb95453357a07fbaf05497ef4e5bb83d92b96caf62c1cddaa616
@l0rinc
test: exercise ReadBlock hash‑mismatch path
c8cd9c6 
Ensure `ReadBlock` rejects a block when the tip’s `phashBlock` differs from the expected hash.
@l0rinc
node: avoid recomputing block hash in ReadBlock
8117c16 
Eliminate one SHA‑256 double‑hash computation of the header per block read by reusing the hash for:
* proof‑of‑work verification;
* (optional) integrity check against the supplied hash.
@l0rinc l0rinc force-pushed the l0rinc/optimize-readblock-hash-check branch from 3808095 to 8117c16 Compare May 13, 2025 20:13
@l0rinc l0rinc closed this May 13, 2025
l0rinc pushed a commit that referenced this pull request Jun 25, 2025
@fanquake
lsan: add more Qt suppressions
5be31b2 
Using Clang clang version 20.1.6 (Fedora 20.1.6-9.fc43) and:
```bash
export CC=clang
export CXX=clang++
cmake -B build -DBUILD_GUI=ON -DSANITIZERS=address
cmake --build build
export LSAN_OPTIONS="suppressions=/root/bitcoin/test/sanitizer_suppressions/lsan"
ctest --test-dir build
```

```bash
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 1589ms
********* Finished testing of AddressBookTests *********

=================================================================
==21869==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 88 byte(s) in 1 object(s) allocated from:
    #0 0xaaaab5d5af40 in operator new(unsigned long) (/root/bitcoin/build/bin/test_bitcoin-qt+0x39af40) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872)
    #1 0xffff8c8f56cc in QLayoutPrivate::createWidgetItem(QLayout const*, QWidget*) (/lib64/libQt6Widgets.so.6+0x1a56cc) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa)
    #2 0xffff8c8d2f90 in QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) (/lib64/libQt6Widgets.so.6+0x182f90) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa)
    #3 0xaaaab5fc7188 in SendCoinsDialog::addEntry() /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:596:18
    #4 0xaaaab5fc4eec in SendCoinsDialog::SendCoinsDialog(PlatformStyle const*, QWidget*) /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:84:5
    #5 0xaaaab5da67ac in (anonymous namespace)::MiniGUI::MiniGUI(interfaces::Node&, PlatformStyle const*) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:235:75
    #6 0xaaaab5da2000 in (anonymous namespace)::TestGUI(interfaces::Node&, std::shared_ptr<wallet::CWallet> const&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:270:13
    #7 0xaaaab5d9ebc8 in (anonymous namespace)::TestGUI(interfaces::Node&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:453:5
    #8 0xaaaab5d9ebc8 in WalletTests::walletTests() /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:475:5
    #9 0xffff8b1c5314 in QMetaMethodInvoker::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195314) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7)
    #10 0xffff8b1c5dc8 in QMetaMethod::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195dc8) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7)
    #11 0xffff8cf57c54  (/lib64/libQt6Test.so.6+0x27c54) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
    #12 0xffff8cf5fa18  (/lib64/libQt6Test.so.6+0x2fa18) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
    #13 0xffff8cf6067c  (/lib64/libQt6Test.so.6+0x3067c) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
    #14 0xffff8cf610a4  (/lib64/libQt6Test.so.6+0x310a4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
    #15 0xffff8cf61aa4 in QTest::qRun() (/lib64/libQt6Test.so.6+0x31aa4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
    #16 0xffff8cf61eb4 in QTest::qExec(QObject*, int, char**) (/lib64/libQt6Test.so.6+0x31eb4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
    #17 0xaaaab5d7d77c in main /root/bitcoin/build/src/qt/test/./qt/test/test_main.cpp:95:30
    #18 0xffff8aad6398 in __libc_start_call_main (/lib64/libc.so.6+0x26398) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
    #19 0xffff8aad6478 in __libc_start_main@GLIBC_2.17 (/lib64/libc.so.6+0x26478) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
    #20 0xaaaab5c74cac in _start (/root/bitcoin/build/bin/test_bitcoin-qt+0x2b4cac) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872)
```

This happens when building using depends:
```bash
Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0xaaaabdbe86f8 in malloc (/root/bitcoin/build/bin/test_bitcoin-qt+0x4386f8) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #1 0xfbff97f8c164  (<unknown module>)
    #2 0xaaaabf0cfaa4 in QDBusConnectionPrivate::QDBusConnectionPrivate() (/root/bitcoin/build/bin/test_bitcoin-qt+0x191faa4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #3 0xaaaabf0c9e30 in QDBusConnectionManager::doConnectToStandardBus(QDBusConnection::BusType, QString const&, bool) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919e30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #4 0xaaaabf0cb0e4 in QtPrivate::QCallableObject<QDBusConnectionPrivate* (QDBusConnectionManager::*)(QDBusConnection::BusType, QString const&, bool), QtPrivate::List<QDBusConnection::BusType&, QString const&, bool&>, QDBusConnectionPrivate*>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x191b0e4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #5 0xaaaabf5cbaf0 in QObject::event(QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e1baf0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #6 0xaaaabf5a4ce0 in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df4ce0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #7 0xaaaabf5a486c in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df486c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #8 0xaaaabf5a575c in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df575c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #9 0xaaaabf66b858 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1ebb858) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #10 0xaaaabf5a9e3c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df9e3c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #11 0xaaaabf632a44 in QThread::exec() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e82a44) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #12 0xaaaabf0c9bd0 in QDBusConnectionManager::run() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919bd0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #13 0xaaaabf669c30 in QThreadPrivate::start(void*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1eb9c30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
    #14 0xaaaabdbe5f2c in asan_thread_start(void*) asan_interceptors.cpp.o
    #15 0xffff99538608 in thread_start (/lib64/libc.so.6+0xf8608) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)

SUMMARY: AddressSanitizer: 3592 byte(s) leaked in 37 allocation(s).
```
l0rinc pushed a commit that referenced this pull request Jun 25, 2025
@fanquake
Merge bitcoin#32780: lsan: add more Qt suppressions
67e6746 
5be31b2 lsan: add more Qt suppressions (fanquake)

Pull request description:

  Using Clang clang version 20.1.6 (Fedora 20.1.6-9.fc43) and:
  ```bash
  export CC=clang
  export CXX=clang++
  cmake -B build -DBUILD_GUI=ON -DSANITIZERS=address
  cmake --build build
  export LSAN_OPTIONS="suppressions=/root/bitcoin/test/sanitizer_suppressions/lsan"
  ctest --test-dir build
  ```

  ```bash
  Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 1589ms
  ********* Finished testing of AddressBookTests *********

  =================================================================
  ==21869==ERROR: LeakSanitizer: detected memory leaks

  Direct leak of 88 byte(s) in 1 object(s) allocated from:
      #0 0xaaaab5d5af40 in operator new(unsigned long) (/root/bitcoin/build/bin/test_bitcoin-qt+0x39af40) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872)
      #1 0xffff8c8f56cc in QLayoutPrivate::createWidgetItem(QLayout const*, QWidget*) (/lib64/libQt6Widgets.so.6+0x1a56cc) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa)
      #2 0xffff8c8d2f90 in QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) (/lib64/libQt6Widgets.so.6+0x182f90) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa)
      #3 0xaaaab5fc7188 in SendCoinsDialog::addEntry() /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:596:18
      #4 0xaaaab5fc4eec in SendCoinsDialog::SendCoinsDialog(PlatformStyle const*, QWidget*) /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:84:5
      #5 0xaaaab5da67ac in (anonymous namespace)::MiniGUI::MiniGUI(interfaces::Node&, PlatformStyle const*) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:235:75
      #6 0xaaaab5da2000 in (anonymous namespace)::TestGUI(interfaces::Node&, std::shared_ptr<wallet::CWallet> const&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:270:13
      #7 0xaaaab5d9ebc8 in (anonymous namespace)::TestGUI(interfaces::Node&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:453:5
      #8 0xaaaab5d9ebc8 in WalletTests::walletTests() /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:475:5
      #9 0xffff8b1c5314 in QMetaMethodInvoker::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195314) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7)
      #10 0xffff8b1c5dc8 in QMetaMethod::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195dc8) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7)
      #11 0xffff8cf57c54  (/lib64/libQt6Test.so.6+0x27c54) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
      #12 0xffff8cf5fa18  (/lib64/libQt6Test.so.6+0x2fa18) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
      #13 0xffff8cf6067c  (/lib64/libQt6Test.so.6+0x3067c) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
      #14 0xffff8cf610a4  (/lib64/libQt6Test.so.6+0x310a4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
      #15 0xffff8cf61aa4 in QTest::qRun() (/lib64/libQt6Test.so.6+0x31aa4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
      #16 0xffff8cf61eb4 in QTest::qExec(QObject*, int, char**) (/lib64/libQt6Test.so.6+0x31eb4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
      #17 0xaaaab5d7d77c in main /root/bitcoin/build/src/qt/test/./qt/test/test_main.cpp:95:30
      #18 0xffff8aad6398 in __libc_start_call_main (/lib64/libc.so.6+0x26398) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
      #19 0xffff8aad6478 in __libc_start_main@GLIBC_2.17 (/lib64/libc.so.6+0x26478) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
      #20 0xaaaab5c74cac in _start (/root/bitcoin/build/bin/test_bitcoin-qt+0x2b4cac) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872)
  ```

  This happens when building using depends:
  ```bash
  Indirect leak of 24 byte(s) in 1 object(s) allocated from:
      #0 0xaaaabdbe86f8 in malloc (/root/bitcoin/build/bin/test_bitcoin-qt+0x4386f8) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #1 0xfbff97f8c164  (<unknown module>)
      #2 0xaaaabf0cfaa4 in QDBusConnectionPrivate::QDBusConnectionPrivate() (/root/bitcoin/build/bin/test_bitcoin-qt+0x191faa4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #3 0xaaaabf0c9e30 in QDBusConnectionManager::doConnectToStandardBus(QDBusConnection::BusType, QString const&, bool) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919e30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #4 0xaaaabf0cb0e4 in QtPrivate::QCallableObject<QDBusConnectionPrivate* (QDBusConnectionManager::*)(QDBusConnection::BusType, QString const&, bool), QtPrivate::List<QDBusConnection::BusType&, QString const&, bool&>, QDBusConnectionPrivate*>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x191b0e4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #5 0xaaaabf5cbaf0 in QObject::event(QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e1baf0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #6 0xaaaabf5a4ce0 in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df4ce0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #7 0xaaaabf5a486c in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df486c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #8 0xaaaabf5a575c in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df575c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #9 0xaaaabf66b858 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1ebb858) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #10 0xaaaabf5a9e3c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df9e3c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #11 0xaaaabf632a44 in QThread::exec() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e82a44) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #12 0xaaaabf0c9bd0 in QDBusConnectionManager::run() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919bd0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #13 0xaaaabf669c30 in QThreadPrivate::start(void*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1eb9c30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
      #14 0xaaaabdbe5f2c in asan_thread_start(void*) asan_interceptors.cpp.o
      #15 0xffff99538608 in thread_start (/lib64/libc.so.6+0xf8608) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)

  SUMMARY: AddressSanitizer: 3592 byte(s) leaked in 37 allocation(s).
  ```

ACKs for top commit:
  maflcko:
    lgtm ACK 5be31b2

Tree-SHA512: 0c33661c7ec83ea9b874c1ee4ee2de513131690287363e216a88560dfb31a59ef563a50af756c86a991583aa64a600a74e20fd5d6a104cf4c0a27532de8d2211
l0rinc pushed a commit that referenced this pull request Jul 28, 2025
@fanquake
Merge bitcoin#33063: util: Revert "common: Close non-std fds before e…
75a5c82 
…xec in RunCommandJSON"

faa1c3e Revert "Merge bitcoin#32343: common: Close non-std fds before exec in RunCommandJSON" (MarcoFalke)

Pull request description:

  After a fork() in a multithreaded program, the child can safely
  call only async-signal-safe functions (see [signal-safety(7)](https://www.man7.org/linux/man-pages/man7/signal-safety.7.html))
  until such time as it calls execv.

  The standard library (`std` namespace) is not async-signal-safe. Also, `throw`, isn't.

  There was an alternative implementation using `readdir` (bitcoin#32529), but that isn't async-signal-safe either, and that implementation was still using `throw`.

  So temporarily revert this feature.

  A follow-up in the future can add it back, using only async-signal-safe functions, or by using a different approach.

  Fixes bitcoin#32524
  Fixes bitcoin#33015
  Fixes bitcoin#32855

  For reference, a failure can manifest in the GCC debug mode:

  * While `fork`ing, a debug mode mutex is held (by any other thread).
  * The `fork`ed child tries to use the stdard libary before `execv` and deadlocks.

  This may look like the following:

  ```
  (gdb) thread apply all bt

  Thread 1 (Thread 0xf58f4b40 (LWP 774911) "b-httpworker.2"):
  #0  0xf7f4f589 in __kernel_vsyscall ()
  #1  0xf79e467e in ?? () from /lib32/libc.so.6
  #2  0xf79eb582 in pthread_mutex_lock () from /lib32/libc.so.6
  #3  0xf7d93bf2 in ?? () from /lib32/libstdc++.so.6
  #4  0xf7d93f36 in __gnu_debug::_Safe_iterator_base::_M_attach(__gnu_debug::_Safe_sequence_base*, bool) () from /lib32/libstdc++.so.6
  #5  0x5668810a in __gnu_debug::_Safe_iterator_base::_Safe_iterator_base (this=0xf58f13ac, __seq=0xf58f13f8, __constant=false) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_base.h:91
  #6  0x56ddfb50 in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::forward_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:162
  #7  0x56ddfacb in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::bidirectional_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:539
  #8  0x56ddfa5b in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::random_access_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:687
  #9  0x56ddd3f6 in std::__debug::vector<int, std::allocator<int> >::begin (this=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/vector:300
  #10 0x57d83701 in subprocess::detail::Child::execute_child (this=0xf58f156c) at ./util/subprocess.h:1372
  #11 0x57d80a7c in subprocess::Popen::execute_process (this=0xf58f1cd8) at ./util/subprocess.h:1231
  #12 0x57d6d2b4 in subprocess::Popen::Popen<subprocess::input, subprocess::output, subprocess::error, subprocess::close_fds> (this=0xf58f1cd8, cmd_args="fake.py enumerate", args=..., args=..., args=..., args=...) at ./util/subprocess.h:964
  #13 0x57d6b597 in RunCommandParseJSON (str_command="fake.py enumerate", str_std_in="") at ./common/run_command.cpp:27
  #14 0x57a90547 in ExternalSigner::Enumerate (command="fake.py", signers=std::__debug::vector of length 0, capacity 0, chain="regtest") at ./external_signer.cpp:28
  #15 0x56defdab in enumeratesigners()::$_0::operator()(RPCHelpMan const&, JSONRPCRequest const&) const (this=0xf58f2ba0, self=..., request=...) at ./rpc/external_signer.cpp:51
  ...
  (truncated, only one thread exists)
  ```

ACKs for top commit:
  fanquake:
    ACK faa1c3e
  darosior:
    ACK faa1c3e

Tree-SHA512: 602da5f2eba08d7fe01ba19baf411e287ae27fe2d4b82f41734e05b7b1d938ce94cc0041e86ba677284fa92838e96ebee687023ff28047e2b036fd9a53567e0a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@l0rinc @hebasto @fanquake