Skip to content

Commit

Permalink
Merge bitcoin#18948: qt: Call setParent() in the parent's context
Browse files Browse the repository at this point in the history
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 dashpay#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 dashpay#24, name = 'QThread', stop reason = signal SIGABRT
    * frame #0: 0x00007ffff794518b libc.so.6`__GI_raise(sig=2) at raise.c:51:1
      frame dashpay#1: 0x00007ffff7924859 libc.so.6`__GI_abort at abort.c:79:7
      frame dashpay#2: 0x0000555556508ec4 bitcoin-qt`::qt_message_fatal((null)=<unavailable>, context=<unavailable>, message=<unavailable>) at qlogging.cpp:1690:15
      frame dashpay#3: 0x00005555565099cf bitcoin-qt`QMessageLogger::fatal(this=<unavailable>, msg=<unavailable>) const at qlogging.cpp:796:21
      frame dashpay#4: 0x000055555650479d bitcoin-qt`qt_assert_x(where=<unavailable>, what=<unavailable>, file=<unavailable>, line=<unavailable>) at qglobal.cpp:3088:46
      frame dashpay#5: 0x0000555556685733 bitcoin-qt`QCoreApplicationPrivate::checkReceiverThread(receiver=0x0000555557b27510) at qcoreapplication.cpp:557:5
      frame dashpay#6: 0x00005555567ced86 bitcoin-qt`QApplication::notify(this=0x00007fffffffd4a0, receiver=0x0000555557b27510, e=0x00007fff9a7f8ce0) at qapplication.cpp:2956:27
      frame dashpay#7: 0x0000555556685d31 bitcoin-qt`QCoreApplication::notifyInternal2(receiver=0x0000555557b27510, event=0x00007fff9a7f8ce0) at qcoreapplication.cpp:1024:24
      frame dashpay#8: 0x00005555566c9224 bitcoin-qt`QObjectPrivate::setParent_helper(QObject*) [inlined] QCoreApplication::sendEvent(event=<unavailable>, receiver=<unavailable>) at qcoreapplication.h:233:59
      frame dashpay#9: 0x00005555566c9210 bitcoin-qt`QObjectPrivate::setParent_helper(this=0x00007fff85855260, o=0x0000555557b27510) at qobject.cpp:2036
      frame dashpay#10: 0x00005555566c9b41 bitcoin-qt`QObject::setParent(this=<unavailable>, parent=<unavailable>) at qobject.cpp:1980:24
      frame dashpay#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
  • Loading branch information
jonasschnelli authored and vijaydasmp committed Feb 16, 2024
1 parent fa4ced9 commit 05a2b54
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
14 changes: 14 additions & 0 deletions src/qt/guiutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,20 @@ namespace GUIUtil
return string.split(separator, QString::SkipEmptyParts);
#endif
}

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

} // namespace GUIUtil

#endif // BITCOIN_QT_GUIUTIL_H
17 changes: 13 additions & 4 deletions src/qt/walletcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,20 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
}

// Instantiate model and register it.
WalletModel* wallet_model = new WalletModel(std::move(wallet), m_client_model, nullptr);
// Handler callback runs in a different thread so fix wallet model thread affinity.
WalletModel* wallet_model = new WalletModel(std::move(wallet), m_client_model,
nullptr /* required for the following moveToThread() call */);

// Move WalletModel object to the thread that created the WalletController
// object (GUI main thread), instead of the current thread, which could be
// an outside wallet thread or RPC thread sending a LoadWallet notification.
// This ensures queued signals sent to the WalletModel object will be
// handled on the GUI event loop.
wallet_model->moveToThread(thread());
wallet_model->setParent(this);
// setParent(parent) must be called in the thread which created the parent object. More details in #18948.
GUIUtil::ObjectInvoke(this, [wallet_model, this] {
wallet_model->setParent(this);
}, GUIUtil::blockingGUIThreadConnection());

m_wallets.push_back(wallet_model);

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

// Notify walletAdded signal on the GUI thread.
Q_EMIT walletAdded(wallet_model);

return wallet_model;
Expand Down

0 comments on commit 05a2b54

Please sign in to comment.