From e354bdc5cbe33621def2b200f01d6376291570fc Mon Sep 17 00:00:00 2001 From: Dave Andrews Date: Mon, 22 Aug 2016 22:34:06 -0400 Subject: Fix asynchronous method calls on QObjects in different threads Use QMetaMethod::invoke without a return for void method calls, which allows making asynchronous method calls onto QObjects in different affinities than the QWebChannel that's emitting them. Also adds a unit test called testAsyncObject that intentionally places a QObject in a different affinity and then tests calls into it from the QWebChannel's synchronous publisher. Task-number: QTBUG-47678 Change-Id: I6c35ee54f764c0fc1b0431fb0774aa7e75039abf Reviewed-by: Dave Andrews Reviewed-by: Milian Wolff --- src/webchannel/qmetaobjectpublisher.cpp | 24 ++++++++++------- tests/auto/webchannel/tst_webchannel.cpp | 45 ++++++++++++++++++++++++++++++++ tests/auto/webchannel/tst_webchannel.h | 11 ++++++++ 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/webchannel/qmetaobjectpublisher.cpp b/src/webchannel/qmetaobjectpublisher.cpp index cd6ad70..85a9b35 100644 --- a/src/webchannel/qmetaobjectpublisher.cpp +++ b/src/webchannel/qmetaobjectpublisher.cpp @@ -368,22 +368,26 @@ QVariant QMetaObjectPublisher::invokeMethod(QObject *const object, const int met for (int i = 0; i < qMin(args.size(), method.parameterCount()); ++i) { arguments[i].value = toVariant(args.at(i), method.parameterType(i)); } - // construct QGenericReturnArgument QVariant returnValue; - if (method.returnType() != qMetaTypeId() && method.returnType() != qMetaTypeId()) { + if (method.returnType() == QMetaType::Void) { + // Skip return for void methods (prevents runtime warnings inside Qt), and allows + // QMetaMethod to invoke void-returning methods on QObjects in a different thread. + method.invoke(object, + arguments[0], arguments[1], arguments[2], arguments[3], arguments[4], + arguments[5], arguments[6], arguments[7], arguments[8], arguments[9]); + } else { + QGenericReturnArgument returnArgument(method.typeName(), returnValue.data()); + // Only init variant with return type if its not a variant itself, which would // lead to nested variants which is not what we want. - // Also, skip void-return types for obvious reasons (and to prevent a runtime warning inside Qt). - returnValue = QVariant(method.returnType(), 0); - } - QGenericReturnArgument returnArgument(method.typeName(), returnValue.data()); - - // now we can call the method - method.invoke(object, returnArgument, + if (method.returnType() != QMetaType::QVariant) + returnValue = QVariant(method.returnType(), 0); + method.invoke(object, returnArgument, arguments[0], arguments[1], arguments[2], arguments[3], arguments[4], arguments[5], arguments[6], arguments[7], arguments[8], arguments[9]); - + } + // now we can call the method return returnValue; } diff --git a/tests/auto/webchannel/tst_webchannel.cpp b/tests/auto/webchannel/tst_webchannel.cpp index 5ee26ec..55723ea 100644 --- a/tests/auto/webchannel/tst_webchannel.cpp +++ b/tests/auto/webchannel/tst_webchannel.cpp @@ -368,6 +368,12 @@ void TestWebChannel::testInfoForObject() method.append(obj.metaObject()->indexOfMethod("setObjectProperty(QObject*)")); expected.append(method); } + { + QJsonArray method; + method.append(QStringLiteral("setProp")); + method.append(obj.metaObject()->indexOfMethod("setProp(QString)")); + expected.append(method); + } { QJsonArray method; method.append(QStringLiteral("method1")); @@ -478,6 +484,19 @@ void TestWebChannel::testInfoForObject() property.append(QJsonValue::fromVariant(QVariant::fromValue(obj.returnedObject()))); expected.append(property); } + { + QJsonArray property; + property.append(obj.metaObject()->indexOfProperty("prop")); + property.append(QStringLiteral("prop")); + { + QJsonArray signal; + signal.append(1); + signal.append(obj.metaObject()->indexOfMethod("propChanged(QString)")); + property.append(signal); + } + property.append(QJsonValue::fromVariant(QVariant::fromValue(obj.prop()))); + expected.append(property); + } QCOMPARE(info["properties"].toArray(), expected); } } @@ -699,6 +718,32 @@ void TestWebChannel::testInfiniteRecursion() QJsonObject objectInfo = channel.d_func()->publisher->wrapResult(QVariant::fromValue(&obj), m_dummyTransport).toObject(); } +void TestWebChannel::testAsyncObject() +{ + QWebChannel channel; + channel.connectTo(m_dummyTransport); + + QThread thread; + thread.start(); + + TestObject obj; + obj.moveToThread(&thread); + + QJsonArray args; + args.append(QJsonValue("message")); + + int method = obj.metaObject()->indexOfMethod("setProp(QString)"); + QVERIFY(method != -1); + + QSignalSpy spy(&obj, &TestObject::propChanged); + channel.d_func()->publisher->invokeMethod(&obj, method, args); + QVERIFY(spy.wait()); + QCOMPARE(spy.at(0).at(0).toString(), args.at(0).toString()); + + thread.quit(); + thread.wait(); +} + static QHash createObjects(QObject *parent) { const int num = 100; diff --git a/tests/auto/webchannel/tst_webchannel.h b/tests/auto/webchannel/tst_webchannel.h index 0f5cf1c..70a368c 100644 --- a/tests/auto/webchannel/tst_webchannel.h +++ b/tests/auto/webchannel/tst_webchannel.h @@ -74,6 +74,7 @@ class TestObject : public QObject Q_PROPERTY(QString bar READ bar NOTIFY theBarHasChanged) Q_PROPERTY(QObject * objectProperty READ objectProperty WRITE setObjectProperty NOTIFY objectPropertyChanged) Q_PROPERTY(TestObject * returnedObject READ returnedObject WRITE setReturnedObject NOTIFY returnedObjectChanged) + Q_PROPERTY(QString prop READ prop WRITE setProp NOTIFY propChanged) public: explicit TestObject(QObject *parent = 0) @@ -101,6 +102,11 @@ public: return mReturnedObject; } + QString prop() const + { + return mProp; + } + Q_INVOKABLE void method1() {} protected: @@ -116,6 +122,7 @@ signals: void theBarHasChanged(); void objectPropertyChanged(); void returnedObjectChanged(); + void propChanged(const QString&); public slots: void slot1() {} @@ -133,6 +140,8 @@ public slots: emit objectPropertyChanged(); } + void setProp(const QString&prop) {emit propChanged(mProp=prop);} + protected slots: void slot3() {} @@ -142,6 +151,7 @@ private slots: public: QObject *mObjectProperty; TestObject *mReturnedObject; + QString mProp; }; class BenchObject : public QObject @@ -286,6 +296,7 @@ private slots: void testRemoveUnusedTransports(); void testPassWrappedObjectBack(); void testInfiniteRecursion(); + void testAsyncObject(); void benchClassInfo(); void benchInitializeClients(); -- cgit v1.2.1 From 7cb2d467c3421eced44ae7b887b8738cc68595fc Mon Sep 17 00:00:00 2001 From: Dave Andrews Date: Mon, 22 Aug 2016 22:43:55 -0400 Subject: Fix asynchronous signals from QObjects in different threads Switches the signal listener in QWebChannel from using Qt::DirectConnection to Qt::AutoConnection to relay signals from QObjects in different affinities than the QWebChannel. Also adds a unit test in testAsyncObject() to verify that QWebChannel no longer crashes when receiving such a signal, and that such objects can be added and removed dynamically. Task-number: QTBUG-51366 Change-Id: I51a4886286fec9257a21ea95360c1ea8889a584a Reviewed-by: Dave Andrews Reviewed-by: Milian Wolff --- src/webchannel/signalhandler_p.h | 2 +- tests/auto/webchannel/tst_webchannel.cpp | 40 ++++++++++++++++++++++++++++---- tests/auto/webchannel/tst_webchannel.h | 2 ++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/webchannel/signalhandler_p.h b/src/webchannel/signalhandler_p.h index 971c1c9..0f99c5c 100644 --- a/src/webchannel/signalhandler_p.h +++ b/src/webchannel/signalhandler_p.h @@ -178,7 +178,7 @@ void SignalHandler::connectTo(const QObject *object, const int signalI } // otherwise not yet connected, do so now static const int memberOffset = QObject::staticMetaObject.methodCount(); - QMetaObject::Connection connection = QMetaObject::connect(object, signal.methodIndex(), this, memberOffset + signalIndex, Qt::DirectConnection, 0); + QMetaObject::Connection connection = QMetaObject::connect(object, signal.methodIndex(), this, memberOffset + signalIndex, Qt::AutoConnection, 0); if (!connection) { qWarning() << "SignalHandler: QMetaObject::connect returned false. Unable to connect to" << object << signal.name() << signal.methodSignature(); return; diff --git a/tests/auto/webchannel/tst_webchannel.cpp b/tests/auto/webchannel/tst_webchannel.cpp index 55723ea..2ab820b 100644 --- a/tests/auto/webchannel/tst_webchannel.cpp +++ b/tests/auto/webchannel/tst_webchannel.cpp @@ -374,6 +374,12 @@ void TestWebChannel::testInfoForObject() method.append(obj.metaObject()->indexOfMethod("setProp(QString)")); expected.append(method); } + { + QJsonArray method; + method.append(QStringLiteral("fire")); + method.append(obj.metaObject()->indexOfMethod("fire()")); + expected.append(method); + } { QJsonArray method; method.append(QStringLiteral("method1")); @@ -403,6 +409,12 @@ void TestWebChannel::testInfoForObject() signal.append(obj.metaObject()->indexOfMethod("sig2(QString)")); expected.append(signal); } + { + QJsonArray signal; + signal.append(QStringLiteral("replay")); + signal.append(obj.metaObject()->indexOfMethod("replay()")); + expected.append(signal); + } QCOMPARE(info["signals"].toArray(), expected); } @@ -735,10 +747,30 @@ void TestWebChannel::testAsyncObject() int method = obj.metaObject()->indexOfMethod("setProp(QString)"); QVERIFY(method != -1); - QSignalSpy spy(&obj, &TestObject::propChanged); - channel.d_func()->publisher->invokeMethod(&obj, method, args); - QVERIFY(spy.wait()); - QCOMPARE(spy.at(0).at(0).toString(), args.at(0).toString()); + { + QSignalSpy spy(&obj, &TestObject::propChanged); + channel.d_func()->publisher->invokeMethod(&obj, method, args); + QVERIFY(spy.wait()); + QCOMPARE(spy.at(0).at(0).toString(), args.at(0).toString()); + } + + channel.registerObject("myObj", &obj); + channel.d_func()->publisher->initializeClient(m_dummyTransport); + + QJsonObject connectMessage; + connectMessage["type"] = 7; + connectMessage["object"] = "myObj"; + connectMessage["signal"] = obj.metaObject()->indexOfSignal("replay()"); + channel.d_func()->publisher->handleMessage(connectMessage, m_dummyTransport); + + { + QSignalSpy spy(&obj, &TestObject::replay); + QMetaObject::invokeMethod(&obj, "fire"); + QVERIFY(spy.wait()); + channel.deregisterObject(&obj); + QMetaObject::invokeMethod(&obj, "fire"); + QVERIFY(spy.wait()); + } thread.quit(); thread.wait(); diff --git a/tests/auto/webchannel/tst_webchannel.h b/tests/auto/webchannel/tst_webchannel.h index 70a368c..94dccf5 100644 --- a/tests/auto/webchannel/tst_webchannel.h +++ b/tests/auto/webchannel/tst_webchannel.h @@ -123,6 +123,7 @@ signals: void objectPropertyChanged(); void returnedObjectChanged(); void propChanged(const QString&); + void replay(); public slots: void slot1() {} @@ -141,6 +142,7 @@ public slots: } void setProp(const QString&prop) {emit propChanged(mProp=prop);} + void fire() {emit replay();} protected slots: void slot3() {} -- cgit v1.2.1