Segfault during shutdown in SendCoinsDialog::updateCoinControlState #862

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](/bitcoin-core-gui/0/)  0x00005555558ba7b2 in SendCoinsDialog::updateCoinControlState (
    this=this@entry=0x55555833f710) at ./qt/sendcoinsdialog.cpp:850
[#1](/bitcoin-core-gui/1/)  0x00005555558ba88e in SendCoinsDialog::updateSmartFeeLabel (
    this=0x55555833f710) at ./qt/sendcoinsdialog.cpp:863
[#2](/bitcoin-core-gui/2/)  0x0000555556a99c3e in QObject::event(QEvent*) ()
[#3](/bitcoin-core-gui/3/)  0x0000555555c4e603 in QApplicationPrivate::notify_helper(QObject*, QEvent*)
    ()
[#4](/bitcoin-core-gui/4/)  0x0000555556a6829a in QCoreApplication::notifyInternal2(QObject*, QEvent*)
    ()
[#5](/bitcoin-core-gui/5/)  0x0000555556a6c2bb in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
[#6](/bitcoin-core-gui/6/)  0x0000555556abfd45 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
[#7](/bitcoin-core-gui/7/)  0x000055555636ae12 in QXcbUnixEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
[#8](/bitcoin-core-gui/8/)  0x0000555556a66da6 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
[#9](/bitcoin-core-gui/9/)  0x0000555556a70128 in QCoreApplication::exec() ()
[#10](/bitcoin-core-gui/10/) 0x00005555557bacda in GuiMain (argc=3, argv=0x7fffffffdff8)
    at ./qt/bitcoin.cpp:691
[#11](/bitcoin-core-gui/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](/bitcoin-core-gui/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](/bitcoin-core-gui/13/) 0x00005555557b29f5 in _start ()

Thus, the code seems to be:

https://github.com/bitcoin-core/gui/blob/af3dee0b8d45/src/qt/sendcoinsdialog.cpp#L850

cc @achow101 @furszy

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

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)
...

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.

* 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).