Skip to content

Segfault during shutdown in SendCoinsDialog::updateCoinControlState #862

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
Open
maflcko opened this issue Mar 31, 2025 · 5 comments · May be fixed by #864
Open

Segfault during shutdown in SendCoinsDialog::updateCoinControlState #862

maflcko opened this issue Mar 31, 2025 · 5 comments · May be fixed by #864
Labels
Bug Something isn't working

Comments

@maflcko
Copy link
Contributor

maflcko commented Mar 31, 2025

During IBD, while shutting down the gui, it crashed with Thread 1 "bitcoin-qt" received signal SIGSEGV, Segmentation fault. Version: v29.99.0-af3dee0b8d45

The bt is:

(gdb) bt
#0  0x00005555558ba7b2 in SendCoinsDialog::updateCoinControlState (
    this=this@entry=0x55555833f710) at ./qt/sendcoinsdialog.cpp:850
#1  0x00005555558ba88e in SendCoinsDialog::updateSmartFeeLabel (
    this=0x55555833f710) at ./qt/sendcoinsdialog.cpp:863
#2  0x0000555556a99c3e in QObject::event(QEvent*) ()
#3  0x0000555555c4e603 in QApplicationPrivate::notify_helper(QObject*, QEvent*)
    ()
#4  0x0000555556a6829a in QCoreApplication::notifyInternal2(QObject*, QEvent*)
    ()
#5  0x0000555556a6c2bb in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
#6  0x0000555556abfd45 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
#7  0x000055555636ae12 in QXcbUnixEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
#8  0x0000555556a66da6 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
#9  0x0000555556a70128 in QCoreApplication::exec() ()
#10 0x00005555557bacda in GuiMain (argc=3, argv=0x7fffffffdff8)
    at ./qt/bitcoin.cpp:691
#11 0x00007ffff7867d90 in __libc_start_call_main (
    main=main@entry=0x55555573a580 <main(int, char**)>, argc=argc@entry=3,
    argv=argv@entry=0x7fffffffdff8)
    at ../sysdeps/nptl/libc_start_call_main.h:58
#12 0x00007ffff7867e40 in __libc_start_main_impl (
    main=0x55555573a580 <main(int, char**)>, argc=3, argv=0x7fffffffdff8,
    init=<optimised out>, fini=<optimised out>, rtld_fini=<optimised out>,
    stack_end=0x7fffffffdfe8) at ../csu/libc-start.c:392
#13 0x00005555557b29f5 in _start ()

Thus, the code seems to be:

https://github.com/bitcoin-core/gui/blob/af3dee0b8d45/src/qt/sendcoinsdialog.cpp#L850
@maflcko maflcko added the Bug Something isn't working label Mar 31, 2025
@hebasto
Copy link
Member

hebasto commented Mar 31, 2025

cc @achow101 @furszy

@maflcko
Copy link
Contributor Author

maflcko commented Mar 31, 2025

I guess this is a race from the block tip event (probably optimized out in the bt) and the wallet unloading.

@maflcko
Copy link
Contributor Author

maflcko commented Mar 31, 2025

Steps to reproduce:

  • Apply diff on current master to create more (fake) block tip events for the gui
diff --git a/src/init.cpp b/src/init.cpp
index f35a547c92..7c5fc7f65f 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1885,7 +1885,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     int64_t best_block_time{};
     {
         LOCK(chainman.GetMutex());
-        const auto& tip{*Assert(chainman.ActiveTip())};
+        auto& tip{*Assert(chainman.ActiveTip())};
         LogPrintf("block tree size = %u\n", chainman.BlockIndex().size());
         chain_active_height = tip.nHeight;
         best_block_time = tip.GetBlockTime();
@@ -1898,6 +1898,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             tip_info->header_height = chainman.m_best_header->nHeight;
             tip_info->header_time = chainman.m_best_header->GetBlockTime();
         }
+        scheduler.scheduleEvery([&] { (void)chainman.GetNotifications().blockTip(SynchronizationState::POST_INIT, tip); }, 5ms);
     }
     LogPrintf("nBestHeight = %d\n", chain_active_height);
     if (node.peerman) node.peerman->SetBestBlock(chain_active_height, std::chrono::seconds{best_block_time});
  • Start the gui in a fresh datadir in regtest in a debugger, like valgrind or gdb
valgrind --tool=none ./bld-cmake/bin/bitcoin-qt -datadir=/tmp -regtest

  • Create a wallet

  • Start-stop the GUI in a loop (with the command above), possibly adjust the scheduler interval up or down in the code

  • Wait until it crashes:

==72559== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==72559==  Access not within mapped region at address 0x18



==72559==    at 0x23EBAF: SendCoinsDialog::updateCoinControlState() (./qt/sendcoinsdialog.cpp:850)
==72559==    by 0x23BF53: SendCoinsDialog::updateSmartFeeLabel() (./qt/sendcoinsdialog.cpp:863)
==72559==    by 0x5BB2B62: QObject::event(QEvent*) (qobject.cpp:1347)
...

@furszy
Copy link
Member

furszy commented Apr 3, 2025

If you ever comeback to this, please try this out:

diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
--- a/src/qt/sendcoinsdialog.cpp	(revision 099ae951227616a841219ea11f7fe0b6e567e047)
+++ b/src/qt/sendcoinsdialog.cpp	(date 1743708385724)
@@ -142,7 +142,11 @@
     this->clientModel = _clientModel;
 
     if (_clientModel) {
-        connect(_clientModel, &ClientModel::numBlocksChanged, this, &SendCoinsDialog::updateNumberOfBlocks);
+        connect(clientModel, &ClientModel::numBlocksChanged, this, &SendCoinsDialog::updateNumberOfBlocks);
+    } else { // Receiving a nullptr clientModel indicates that the app is shutting down.
+        // Disconnect signals so we don't attempt to update views during shutdown.
+        // Attempting to update them could crash the app if the methods try to fetch data from the backend models.
+        disconnect(clientModel, &ClientModel::numBlocksChanged, this, &SendCoinsDialog::updateNumberOfBlocks);
     }
 }

Also, another possible solution could be to check for shutdownRequested() inside SendCoinsDialog::updateNumberOfBlocks, but I found that uglier than the suggestion I pasted above.

@furszy furszy linked a pull request Apr 7, 2025 that will close this issue
Open
@pablomartin4btc
Copy link
Contributor 

* Start-stop the GUI in a loop (with the command above), possibly adjust the scheduler interval up or down in the code

I've managed to reproduce the issue on v29, on my Ubuntu 22.04 it happens every time (tried it also on previous versions even with the loop but don't crash).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gui: crash fix, disconnect numBlocksChanged() signal during shutdown furszy/bitcoin-gui
4 participants
@furszy @maflcko @hebasto @pablomartin4btc