Skip to content

Commit ea82816

Browse files
jonasschnelliPastaPastaPasta
authored andcommitted
Merge bitcoin#18948: qt: Call setParent() in the parent's context
8963b2c qt: Improve comments in WalletController::getOrCreateWallet() (Hennadii Stepanov) 5fcfee6 qt: Call setParent() in the parent's context (Hennadii Stepanov) 5659e73 qt: Add ObjectInvoke template function (Hennadii Stepanov) Pull request description: The `setParent(parent)` internally calls `QCoreApplication::sendEvent(parent, QChildEvent)` that implies running in the thread which created the parent object. That is not the case always, and an internal assertion fails in the debug mode. Steps to reproduce this issue on master (007e15d) on Linux Mint 20 (x86_64): ``` $ make -C depends DEBUG=1 $ CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure $ make $ QT_FATAL_WARNINGS=1 lldb src/qt/bitcoin-qt -- --regtest -debug=qt (lldb) target create "src/qt/bitcoin-qt" Current executable set to '/home/hebasto/GitHub/bitcoin/src/qt/bitcoin-qt' (x86_64). (lldb) settings set -- target.run-args "--regtest" "-debug=qt" (lldb) run Process 431562 launched: '/home/hebasto/GitHub/bitcoin/src/qt/bitcoin-qt' (x86_64) # load wallet via GUI Process 431562 stopped * thread #24, name = 'QThread', stop reason = signal SIGABRT frame #0: 0x00007ffff794518b libc.so.6`__GI_raise(sig=2) at raise.c:51:1 (lldb) bt * thread #24, name = 'QThread', stop reason = signal SIGABRT * frame #0: 0x00007ffff794518b libc.so.6`__GI_raise(sig=2) at raise.c:51:1 frame #1: 0x00007ffff7924859 libc.so.6`__GI_abort at abort.c:79:7 frame #2: 0x0000555556508ec4 bitcoin-qt`::qt_message_fatal((null)=<unavailable>, context=<unavailable>, message=<unavailable>) at qlogging.cpp:1690:15 frame #3: 0x00005555565099cf bitcoin-qt`QMessageLogger::fatal(this=<unavailable>, msg=<unavailable>) const at qlogging.cpp:796:21 frame #4: 0x000055555650479d bitcoin-qt`qt_assert_x(where=<unavailable>, what=<unavailable>, file=<unavailable>, line=<unavailable>) at qglobal.cpp:3088:46 frame #5: 0x0000555556685733 bitcoin-qt`QCoreApplicationPrivate::checkReceiverThread(receiver=0x0000555557b27510) at qcoreapplication.cpp:557:5 frame #6: 0x00005555567ced86 bitcoin-qt`QApplication::notify(this=0x00007fffffffd4a0, receiver=0x0000555557b27510, e=0x00007fff9a7f8ce0) at qapplication.cpp:2956:27 frame #7: 0x0000555556685d31 bitcoin-qt`QCoreApplication::notifyInternal2(receiver=0x0000555557b27510, event=0x00007fff9a7f8ce0) at qcoreapplication.cpp:1024:24 frame #8: 0x00005555566c9224 bitcoin-qt`QObjectPrivate::setParent_helper(QObject*) [inlined] QCoreApplication::sendEvent(event=<unavailable>, receiver=<unavailable>) at qcoreapplication.h:233:59 frame #9: 0x00005555566c9210 bitcoin-qt`QObjectPrivate::setParent_helper(this=0x00007fff85855260, o=0x0000555557b27510) at qobject.cpp:2036 frame #10: 0x00005555566c9b41 bitcoin-qt`QObject::setParent(this=<unavailable>, parent=<unavailable>) at qobject.cpp:1980:24 frame #11: 0x0000555555710be8 bitcoin-qt`WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet, std::default_delete<interfaces::Wallet> >) + 2534 ... ``` Fixes bitcoin#18835. ACKs for top commit: ryanofsky: Code review ACK 8963b2c. No changes since last review, just rebase because of conflict on some adjacent lines jonasschnelli: utACK 8963b2c Tree-SHA512: fef615904168717df3d8a0bd85eccc3eef990cc3e66c9fa280c8ef08ea009a7cb5a2a4f868ed0be3c0fe5bf683e8465850b5958deb896fdadd22d296186c9586
1 parent 132aded commit ea82816

File tree

2 files changed

+27
-4
lines changed

2 files changed

+27
-4
lines changed

src/qt/guiutil.h

+14
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,20 @@ namespace GUIUtil
504504
return string.split(separator, QString::SkipEmptyParts);
505505
#endif
506506
}
507+
508+
/**
509+
* Queue a function to run in an object's event loop. This can be
510+
* replaced by a call to the QMetaObject::invokeMethod functor overload after Qt 5.10, but
511+
* for now use a QObject::connect for compatibility with older Qt versions, based on
512+
* https://stackoverflow.com/questions/21646467/how-to-execute-a-functor-or-a-lambda-in-a-given-thread-in-qt-gcd-style
513+
*/
514+
template <typename Fn>
515+
void ObjectInvoke(QObject* object, Fn&& function, Qt::ConnectionType connection = Qt::QueuedConnection)
516+
{
517+
QObject source;
518+
QObject::connect(&source, &QObject::destroyed, object, std::forward<Fn>(function), connection);
519+
}
520+
507521
} // namespace GUIUtil
508522

509523
#endif // BITCOIN_QT_GUIUTIL_H

src/qt/walletcontroller.cpp

+13-4
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,20 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
129129
}
130130

131131
// Instantiate model and register it.
132-
WalletModel* wallet_model = new WalletModel(std::move(wallet), m_client_model, nullptr);
133-
// Handler callback runs in a different thread so fix wallet model thread affinity.
132+
WalletModel* wallet_model = new WalletModel(std::move(wallet), m_client_model,
133+
nullptr /* required for the following moveToThread() call */);
134+
135+
// Move WalletModel object to the thread that created the WalletController
136+
// object (GUI main thread), instead of the current thread, which could be
137+
// an outside wallet thread or RPC thread sending a LoadWallet notification.
138+
// This ensures queued signals sent to the WalletModel object will be
139+
// handled on the GUI event loop.
134140
wallet_model->moveToThread(thread());
135-
wallet_model->setParent(this);
141+
// setParent(parent) must be called in the thread which created the parent object. More details in #18948.
142+
GUIUtil::ObjectInvoke(this, [wallet_model, this] {
143+
wallet_model->setParent(this);
144+
}, GUIUtil::blockingGUIThreadConnection());
145+
136146
m_wallets.push_back(wallet_model);
137147

138148
// WalletModel::startPollBalance needs to be called in a thread managed by
@@ -158,7 +168,6 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
158168
// Re-emit coinsSent signal from wallet model.
159169
connect(wallet_model, &WalletModel::coinsSent, this, &WalletController::coinsSent);
160170

161-
// Notify walletAdded signal on the GUI thread.
162171
Q_EMIT walletAdded(wallet_model);
163172

164173
return wallet_model;

0 commit comments

Comments
 (0)