Skip to content

Commit c33662a

Browse files
committed
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 bitcoin#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
2 parents 7f0f01f + 8963b2c commit c33662a

File tree

2 files changed

+25
-4
lines changed

2 files changed

+25
-4
lines changed

src/qt/guiutil.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,18 @@ namespace GUIUtil
340340
#endif
341341
}
342342

343+
/**
344+
* Queue a function to run in an object's event loop. This can be
345+
* replaced by a call to the QMetaObject::invokeMethod functor overload after Qt 5.10, but
346+
* for now use a QObject::connect for compatibility with older Qt versions, based on
347+
* https://stackoverflow.com/questions/21646467/how-to-execute-a-functor-or-a-lambda-in-a-given-thread-in-qt-gcd-style
348+
*/
349+
template <typename Fn>
350+
void ObjectInvoke(QObject* object, Fn&& function, Qt::ConnectionType connection = Qt::QueuedConnection)
351+
{
352+
QObject source;
353+
QObject::connect(&source, &QObject::destroyed, object, std::forward<Fn>(function), connection);
354+
}
343355

344356
} // namespace GUIUtil
345357

src/qt/walletcontroller.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,20 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
128128
}
129129

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

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

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

163172
return wallet_model;

0 commit comments

Comments
 (0)