gui: segfault unloading and immediately reloading wallet with gui

ryanofsky commented at 6:53 pm on March 16, 2020: member

Calling loadwallet and unloadwallet in a loop with the GUI leads to the segfault

To reproduce

0# Build
1git checkout cf4cb28efcf80c018a7f070c671f43cd172dbd86
2make

0# Start gui
1src/qt/bitcoin-qt -regtest -debug=1 -server=1 -printtoconsole

0# Create test wallet
1src/bitcoin-cli -regtest createwallet test
2src/bitcoin-cli -regtest unloadwallet test

0# Trigger segfault
1while src/bitcoin-cli -regtest loadwallet test && src/bitcoin-cli -regtest unloadwallet test; do true; done

Expected behavior

No segfault

Actual behavior

Segfault

Additional information

Stack trace

 0Thread 1 "bitcoin-qt" received signal SIGSEGV, Segmentation fault.
 10x00007ffff6cd2b7b in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
 2(gdb) bt
 3[#0](/bitcoin-bitcoin/0/)  0x00007ffff6cd2b7b in  () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
 4[#1](/bitcoin-bitcoin/1/)  0x00007ffff6cd3512 in QHeaderView::setSectionResizeMode(int, QHeaderView::ResizeMode) () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
 5[#2](/bitcoin-bitcoin/2/)  0x000055555563f40a in GUIUtil::TableViewLastColumnResizingFixer::setViewHeaderResizeMode(int, QHeaderView::ResizeMode) (this=
 6    0x5555583468b0, logicalIndex=-2, resizeMode=QHeaderView::Interactive) at qt/guiutil.cpp:456
 7[#3](/bitcoin-bitcoin/3/)  0x000055555563f838 in GUIUtil::TableViewLastColumnResizingFixer::TableViewLastColumnResizingFixer(QTableView*, int, int, QObject*) (this=0x5555583468b0, table=0x555558168150, lastColMinimumWidth=120, allColsMinimumWidth=23, parent=0x5555581520e0) at qt/guiutil.cpp:549
 8[#4](/bitcoin-bitcoin/4/)  0x00005555557d6677 in TransactionView::setModel(WalletModel*) (this=0x5555581520e0, _model=0x7fff94001540) at qt/transactionview.cpp:235
 9[#5](/bitcoin-bitcoin/5/)  0x0000555555728aef in WalletView::setWalletModel(WalletModel*) (this=0x555557fc0a30, _walletModel=0x7fff94001540) at qt/walletview.cpp:103
10[#6](/bitcoin-bitcoin/6/)  0x00005555556faf1e in WalletFrame::addWallet(WalletModel*) (this=0x555557078120, walletModel=0x7fff94001540) at qt/walletframe.cpp:54
11[#7](/bitcoin-bitcoin/7/)  0x00005555556127f6 in BitcoinGUI::addWallet(WalletModel*) (this=0x555556e85890, walletModel=0x7fff94001540) at qt/bitcoingui.cpp:637
12[#8](/bitcoin-bitcoin/8/)  0x0000555555624456 in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<WalletModel*>, void, void (BitcoinGUI::*)(WalletModel*)>::call(void (BitcoinGUI::*)(WalletModel*), BitcoinGUI*, void**) (f=
13    (void (BitcoinGUI::*)(BitcoinGUI * const, WalletModel *)) 0x5555556127b0 <BitcoinGUI::addWallet(WalletModel*)>, o=0x555556e85890, arg=0x7fff957a43b0)
14    at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobjectdefs_impl.h:136
15[#9](/bitcoin-bitcoin/9/)  0x0000555555624395 in QtPrivate::FunctionPointer<void (BitcoinGUI::*)(WalletModel*)>::call<QtPrivate::List<WalletModel*>, void>(void (BitcoinGUI::*)(WalletModel*), BitcoinGUI*, void**) (f=
16    (void (BitcoinGUI::*)(BitcoinGUI * const, WalletModel *)) 0x5555556127b0 <BitcoinGUI::addWallet(WalletModel*)>, o=0x555556e85890, arg=0x7fff957a43b0)
17    at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobjectdefs_impl.h:169
18[#10](/bitcoin-bitcoin/10/) 0x0000555555624263 in QtPrivate::QSlotObject<void (BitcoinGUI::*)(WalletModel*), QtPrivate::List<WalletModel*>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (which=1, this_=0x555556cd4fd0, r=0x555556e85890, a=0x7fff957a43b0, ret=0x0)
19    at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobject_impl.h:120
20[#11](/bitcoin-bitcoin/11/) 0x00007ffff5d2a0c2 in QObject::event(QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
21[#12](/bitcoin-bitcoin/12/) 0x00007ffff6ab775b in QWidget::event(QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
22[#13](/bitcoin-bitcoin/13/) 0x00007ffff6bcac6b in QMainWindow::event(QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
23[#14](/bitcoin-bitcoin/14/) 0x00007ffff6a7883c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
24[#15](/bitcoin-bitcoin/15/) 0x00007ffff6a80104 in QApplication::notify(QObject*, QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
25[#16](/bitcoin-bitcoin/16/) 0x00007ffff5cfa8d8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
26[#17](/bitcoin-bitcoin/17/) 0x00007ffff5cfd04d in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
27[#18](/bitcoin-bitcoin/18/) 0x00007ffff5d54263 in  () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
28[#19](/bitcoin-bitcoin/19/) 0x00007ffff1a08417 in g_main_context_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
29[#20](/bitcoin-bitcoin/20/) 0x00007ffff1a08650 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
30[#21](/bitcoin-bitcoin/21/) 0x00007ffff1a086dc in g_main_context_iteration () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
31[#22](/bitcoin-bitcoin/22/) 0x00007ffff5d5388f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
32[#23](/bitcoin-bitcoin/23/) 0x00007ffff5cf890a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
33[#24](/bitcoin-bitcoin/24/) 0x00007ffff5d019b4 in QCoreApplication::exec() () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
34[#25](/bitcoin-bitcoin/25/) 0x00005555555f6482 in GuiMain(int, char**) (argc=5, argv=0x7fffffffd418) at qt/bitcoin.cpp:586
35[#26](/bitcoin-bitcoin/26/) 0x00005555555f0e5f in main(int, char**) (argc=5, argv=0x7fffffffd418) at qt/main.cpp:19

This is caused by WalletModel pointer being used after it is deleted. Issue was originally reported #18338 (comment)

ryanofsky added the label Bug on Mar 16, 2020

ryanofsky commented at 7:11 pm on March 16, 2020: member

The steps to reproduce this bug are essentially same steps to reproduce bug reported #16307 (comment), but I think bugs are different. #16307 is an older bug that happens with both bitcoind and bitcoin-qt and is fixed by #18338. This issue is a newer bug specific to bitcoin-qt probably caused by #16261 or another newer GUI-specific wallet loading change

fanquake added the label GUI on Mar 16, 2020

promag commented at 4:23 pm on March 17, 2020: member

@ryanofsky mind testing https://github.com/bitcoin/bitcoin/pull/18338/commits/d71f3b778183c19037efcecc39096182f3a2ef68?

ryanofsky commented at 5:00 pm on March 17, 2020: member

Segfault does happen with d71f3b778183c19037efcecc39096182f3a2ef68. Only difference is a slightly simpler stack trace (QSlotObject FunctionPointer stuff replaced by qt_static_metacall), see below. @promag, have you tried to reproduce the problem with the steps in the description #18362#issue-582531551? The crash happens instantly and reliably for me with first load and unloadwallet rpc calls succeeding and then the crash immediately happening during the second loadwallet call.

I tried a few different hacky fixes for this yesterday but nothing simple seemed to work. I couldn’t figure out where wallet model pointers should be deleted, but the place they are deleted right now is definitely too early:

https://github.com/bitcoin/bitcoin/blob/1d64dfe4fa050a81e4f9b665f2fad2180401cc6e/src/qt/walletcontroller.cpp#L150-L152

If there isn’t a predetermined sequence for unloading wallets in the gui, switching from a raw pointer that gets deleted to a shared pointer that gets reset could be a better way to go there.

 00x00007ffff6cd2b7b in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
 1(gdb) bt
 2[#0](/bitcoin-bitcoin/0/)  0x00007ffff6cd2b7b in  () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
 3[#1](/bitcoin-bitcoin/1/)  0x00007ffff6cd3512 in QHeaderView::setSectionResizeMode(int, QHeaderView::ResizeMode) () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
 4[#2](/bitcoin-bitcoin/2/)  0x00005555555f5395 in GUIUtil::TableViewLastColumnResizingFixer::setViewHeaderResizeMode(int, QHeaderView::ResizeMode) (this=<optimized out>, logicalIndex=<optimized out>, resizeMode=<optimized out>) at qt/guiutil.cpp:456
 5[#3](/bitcoin-bitcoin/3/)  0x00005555555f57e8 in GUIUtil::TableViewLastColumnResizingFixer::TableViewLastColumnResizingFixer(QTableView*, int, int, QObject*) (this=0x555557b77b00, table=0x555557992fa0, lastColMinimumWidth=120, allColsMinimumWidth=23, parent=<optimized out>) at qt/guiutil.cpp:549
 6[#4](/bitcoin-bitcoin/4/)  0x00005555556cb9e3 in TransactionView::setModel(WalletModel*) (this=0x55555797d140, _model=_model@entry=0x7fff94001540) at qt/transactionview.cpp:235
 7[#5](/bitcoin-bitcoin/5/)  0x000055555566b237 in WalletView::setWalletModel(WalletModel*) (this=this@entry=0x55555788ba60, _walletModel=_walletModel@entry=0x7fff94001540) at qt/walletview.cpp:103
 8[#6](/bitcoin-bitcoin/6/)  0x000055555565d2cc in WalletFrame::addWallet(WalletModel*) (this=0x555556d6ba50, walletModel=0x7fff94001540) at qt/walletframe.cpp:54
 9[#7](/bitcoin-bitcoin/7/)  0x00005555555e7344 in BitcoinGUI::addWallet(WalletModel*) (this=0x5555565f9260, walletModel=<optimized out>) at qt/bitcoingui.cpp:637
10[#8](/bitcoin-bitcoin/8/)  0x00007ffff5d2966f in QMetaObject::activate(QObject*, int, int, void**) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
11[#9](/bitcoin-bitcoin/9/)  0x0000555555674f3f in WalletController::walletAdded(WalletModel*) (this=<optimized out>, _t1=<optimized out>) at qt/moc_walletcontroller.cpp:155
12[#10](/bitcoin-bitcoin/10/) 0x00005555556753ac in WalletController::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=0x5555564514d0, _c=<optimized out>, _id=<optimized out>, _a=0x7fff957a4510) at qt/moc_walletcontroller.cpp:84
13[#11](/bitcoin-bitcoin/11/) 0x00007ffff5d2a0c2 in QObject::event(QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
14[#12](/bitcoin-bitcoin/12/) 0x00007ffff6a7883c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
15[#13](/bitcoin-bitcoin/13/) 0x00007ffff6a80104 in QApplication::notify(QObject*, QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
16[#14](/bitcoin-bitcoin/14/) 0x00007ffff5cfa8d8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
17[#15](/bitcoin-bitcoin/15/) 0x00007ffff5cfd04d in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
18[#16](/bitcoin-bitcoin/16/) 0x00007ffff5d54263 in  () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
19[#17](/bitcoin-bitcoin/17/) 0x00007ffff1a08417 in g_main_context_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
20[#18](/bitcoin-bitcoin/18/) 0x00007ffff1a08650 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
21[#19](/bitcoin-bitcoin/19/) 0x00007ffff1a086dc in g_main_context_iteration () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
22[#20](/bitcoin-bitcoin/20/) 0x00007ffff5d5388f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
23[#21](/bitcoin-bitcoin/21/) 0x00007ffff5cf890a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
24[#22](/bitcoin-bitcoin/22/) 0x00007ffff5d019b4 in QCoreApplication::exec() () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
25[#23](/bitcoin-bitcoin/23/) 0x00005555555db5ef in GuiMain(int, char**) (argc=<optimized out>, argv=<optimized out>) at qt/bitcoin.cpp:586
26[#24](/bitcoin-bitcoin/24/) 0x00007ffff3cafb97 in __libc_start_main (main=
27    0x5555555c7720 <main(int, char**)>, argc=5, argv=0x7fffffffd418, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd408)
28    at ../csu/libc-start.c:310
29[#25](/bitcoin-bitcoin/25/) 0x00005555555d554a in _start ()

ryanofsky commented at 8:06 pm on March 17, 2020: member

Did some more testing, and this segfault started happening with #15101. Segfault happens at #15101 merge commit 63144335becb705a233b32fb8d63f37b7d62071a and doesn’t happen at previous commit cd42553b1178a48a16017eff0b70669c84c3895c

At cd42553b1178a48a16017eff0b70669c84c3895c right before #15101, calling unloadwallet just blocks forever. There isn’t even a deadlock. unloadwallet is just stuck waiting for the shared_ptr to be deleted, which I guess never happens because the GUI doesn’t release its reference

promag commented at 11:30 pm on March 19, 2020: member

The problem is in https://github.com/bitcoin/bitcoin/blob/5bf45fe2a9642f8ae8f8a12bcbf8f8b4770421ad/src/qt/walletcontroller.cpp#L119-L130

Note that unload signal is emitted by the wallet model instance which is then deleted in removeAndDeleteWallet. @hebasto please see/test https://github.com/bitcoin/bitcoin/pull/18338/commits/838ea490dd2641e746f870182822dcdf060caa89 (included in #18338).

fanquake deleted a comment on Mar 19, 2020

ryanofsky commented at 2:35 am on March 20, 2020: member

Confirmed 838ea490dd2641e746f870182822dcdf060caa89 fixes the issue for me. Nice to see this resolved so simply!

MarcoFalke commented at 7:16 pm on April 22, 2020: member

Is this fixed?

ryanofsky commented at 8:38 pm on April 27, 2020: member

Is this fixed?

Yes, forgot about this. Will close

ryanofsky closed this on Apr 27, 2020

DrahtBot locked this on Feb 15, 2022

gui: segfault unloading and immediately reloading wallet with gui #18362