Invalid/wrong dynamic calls aren’t verified by the compiler. This PR asserts those dynamic calls. Once we bump Qt to at least 5.10 these can be refactored to use the invokeMethod overload that allows connecting to lambdas or member pointers, which are compile checked.
For reference, one of the overloaded versions is https://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-5.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Once we bump Qt to at least 5.10 these can be refactored to use the
invokeMethodoverload that allows connecting to lambdas or member pointers, which are compile checked.
IMO, in the long run it will be better to get rid of QMetaObject::invokeMethod completely, because QMetaObject class
… is not normally required for application programming…
@laanwj yes, from the docs:
Invokes the member (a signal or a slot name) on the object obj. Returns true if the member could be invoked. Returns false if there is no such member or the parameters did not match.
such as a full queue
Is this even possible?
Is this even possible?
I don’t think so, not sure, my experience (with accidentally not rate-limiting notifications) is that the queue is very deep at least but thanks for quoting the doc that’s good enough for me.
concept and code review ACK 2fd7d0b9081c9add34277e94e8ad21ce22ee2d61
Tested on Debian 9.9 (system Qt 5.7.1):
0$ ./src/qt/bitcoin-qt -regtest -nowallet -debug=qt
Open a wallet from menu…
0bitcoin-qt: qt/walletcontroller.cpp:125: WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet>): Assertion `invoked' failed.
1Aborted
0$ tail -5 ~/.bitcoin/regtest/debug.log
12019-07-08T16:14:33Z [rpc one] nFileVersion = 189900
22019-07-08T16:14:33Z [rpc one] Keys: 2001 plaintext, 0 encrypted, 2001 w/ metadata, 2001 total. Unknown wallet records: 0
32019-07-08T16:14:33Z [rpc one] Wallet completed loading in 97ms
42019-07-08T16:14:33Z GUI: TransactionTablePriv::refreshWallet
52019-07-08T16:14:33Z GUI: QMetaMethod::invoke: Unable to handle unregistered datatype 'WalletModel*'
WalletModel* here:
https://github.com/bitcoin/bitcoin/blob/2679bb8919b5089f8067ccfd94f766747b8df671/src/qt/bitcoin.cpp#L64-L68
and test again.
@promag the latter is enough:
0diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
1index 2fdbcca04..b7740d540 100644
2--- a/src/qt/bitcoin.cpp
3+++ b/src/qt/bitcoin.cpp
4@@ -454,6 +454,7 @@ int GuiMain(int argc, char* argv[])
5 qRegisterMetaType< CAmount >("CAmount");
6 qRegisterMetaType< std::function<void()> >("std::function<void()>");
7 qRegisterMetaType<QMessageBox::Icon>("QMessageBox::Icon");
8+ qRegisterMetaType< WalletModel* >();
9 /// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these
10 // Command-line options take precedence:
11 node->setupServerArgs();
It works.