QHostInfo: Always post results through the event loop to the receiver

Lookups performed via QHostInfoRunnable must not synchronously call
the user-code's receiver objects, as that would execute user-code in
the wrong thread. Instead, post a metacall event through the event
loop of the receiver object, or the thread that initiated the lookup.

This was done correctly for the trivial cases of empty host name or
cached results, so the code generally existed. By moving it from a
global function into a member function of QHostInfoResult, we can
simply access the required data to construct and post the event.

As we process that posted event, we need to check that the context
object (which is already guarded via QPointer) is still alive, if
we had one in the first place. If we had one, and it's deleted, then
abort.

[ChangeLog][QtNetwork][QHostInfo] Functors used in the lookupHost
overloads are now called correctly in the thread of the context object.
When used without context object, the thread that initiates the lookup
will run the functor, and is required to run an event loop.

Change-Id: I9b38d4f9a23cfc4d9e07bc72de2d2cefe5d0d033
Fixes: QTBUG-76276
Reviewed-by: Edward Welbourne <[email protected]>
Reviewed-by: Mårten Nordheim <[email protected]>

src/network/kernel/qhostinfo.cpp

@@ -106,11 +106,39 @@ int get_signal_index()
     return signal_index + QMetaObjectPrivate::signalOffset(senderMetaObject);
 }
 
-void emit_results_ready(const QHostInfo &hostInfo, const QObject *receiver,
-                        QtPrivate::QSlotObjectBase *slotObj)
+}
+
+/*
+    The calling thread is likely the one that executes the lookup via
+    QHostInfoRunnable. Unless we operate with a queued connection already,
+    posts the QHostInfo to a dedicated QHostInfoResult object that lives in
+    the same thread as the user-provided receiver, or (if there is none) in
+    the thread that made the call to lookupHost. That QHostInfoResult object
+    then calls the user code in the correct thread.
+
+    The 'result' object deletes itself (via deleteLater) when the metacall
+    event is received.
+*/
+void QHostInfoResult::postResultsReady(const QHostInfo &info)
 {
+    // queued connection will take care of dispatching to right thread
+    if (!slotObj) {
+        emitResultsReady(info);
+        return;
+    }
     static const int signal_index = get_signal_index();
+
+    // we used to have a context object, but it's already destroyed
+    if (withContextObject && !receiver)
+        return;
+
+    /* QHostInfoResult c'tor moves the result object to the thread of receiver.
+       If we don't have a receiver, then the result object will not live in a
+       thread that runs an event loop - so move it to this' thread, which is the thread
+       that initiated the lookup, and required to have a running event loop. */
     auto result = new QHostInfoResult(receiver, slotObj);
+    if (!receiver)
+        result->moveToThread(thread());
     Q_CHECK_PTR(result);
     const int nargs = 2;
     auto types = reinterpret_cast<int *>(malloc(nargs * sizeof(int)));
@@ -120,15 +148,13 @@ void emit_results_ready(const QHostInfo &hostInfo, const QObject *receiver,
     auto args = reinterpret_cast<void **>(malloc(nargs * sizeof(void *)));
     Q_CHECK_PTR(args);
     args[0] = 0;
-    args[1] = QMetaType::create(types[1], &hostInfo);
+    args[1] = QMetaType::create(types[1], &info);
     Q_CHECK_PTR(args[1]);
     auto metaCallEvent = new QMetaCallEvent(slotObj, nullptr, signal_index, nargs, types, args);
     Q_CHECK_PTR(metaCallEvent);
     qApp->postEvent(result, metaCallEvent);
 }
 
-}
-
 /*!
     \class QHostInfo
     \brief The QHostInfo class provides static functions for host name lookups.
@@ -335,6 +361,10 @@ int QHostInfo::lookupHost(const QString &name, QObject *receiver,
     ready, the \a functor is called with a QHostInfo argument. The
     QHostInfo object can then be inspected to get the results of the
     lookup.
+
+    The \a functor will be run in the thread that makes the call to lookupHost;
+    that thread must have a running Qt event loop.
+
     \note There is no guarantee on the order the signals will be emitted
     if you start multiple requests with lookupHost().
 
@@ -632,7 +662,8 @@ int QHostInfo::lookupHostImpl(const QString &name,
         QHostInfo hostInfo(id);
         hostInfo.setError(QHostInfo::HostNotFound);
         hostInfo.setErrorString(QCoreApplication::translate("QHostInfo", "No host name given"));
-        emit_results_ready(hostInfo, receiver, slotObj);
+        QHostInfoResult result(receiver, slotObj);
+        result.postResultsReady(hostInfo);
         return id;
     }
 
@@ -646,7 +677,8 @@ int QHostInfo::lookupHostImpl(const QString &name,
             QHostInfo info = manager->cache.get(name, &valid);
             if (valid) {
                 info.setLookupId(id);
-                emit_results_ready(info, receiver, slotObj);
+                QHostInfoResult result(receiver, slotObj);
+                result.postResultsReady(info);
                 return id;
             }
         }
@@ -707,7 +739,7 @@ void QHostInfoRunnable::run()
 
     // signal emission
     hostInfo.setLookupId(id);
-    resultEmitter.emitResultsReady(hostInfo);
+    resultEmitter.postResultsReady(hostInfo);
 
 #if QT_CONFIG(thread)
     // now also iterate through the postponed ones
@@ -720,7 +752,7 @@ void QHostInfoRunnable::run()
             QHostInfoRunnable* postponed = *it;
             // we can now emit
             hostInfo.setLookupId(postponed->id);
-            postponed->resultEmitter.emitResultsReady(hostInfo);
+            postponed->resultEmitter.postResultsReady(hostInfo);
             delete postponed;
         }
         manager->postponedLookups.erase(partitionBegin, partitionEnd);

src/network/kernel/qhostinfo_p.h

@@ -84,23 +84,30 @@ class QHostInfoResult : public QObject
 
     QPointer<const QObject> receiver = nullptr;
     QtPrivate::QSlotObjectBase *slotObj = nullptr;
+    const bool withContextObject = false;
 
 public:
     QHostInfoResult() = default;
     QHostInfoResult(const QObject *receiver, QtPrivate::QSlotObjectBase *slotObj) :
         receiver(receiver),
-        slotObj(slotObj)
+        slotObj(slotObj),
+        withContextObject(slotObj && receiver)
     {
         connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this,
                 &QObject::deleteLater);
         if (slotObj && receiver)
             moveToThread(receiver->thread());
     }
 
+    void postResultsReady(const QHostInfo &info);
+
 public Q_SLOTS:
     inline void emitResultsReady(const QHostInfo &info)
     {
         if (slotObj) {
+            // we used to have a context object, but it's already destroyed
+            if (withContextObject && !receiver)
+                return;
             QHostInfo copy = info;
             void *args[2] = { 0, reinterpret_cast<void *>(&copy) };
             slotObj->call(const_cast<QObject*>(receiver.data()), args);

tests/auto/network/kernel/qhostinfo/tst_qhostinfo.cpp

@@ -92,6 +92,7 @@ private slots:
     void lookupIPv6();
     void lookupConnectToFunctionPointer_data();
     void lookupConnectToFunctionPointer();
+    void lookupConnectToFunctionPointerDeleted();
     void lookupConnectToLambda_data();
     void lookupConnectToLambda();
     void reverseLookup_data();
@@ -361,6 +362,17 @@ void tst_QHostInfo::lookupConnectToFunctionPointer()
     QCOMPARE(tmp.join(' '), expected.join(' '));
 }
 
+void tst_QHostInfo::lookupConnectToFunctionPointerDeleted()
+{
+    {
+        QObject contextObject;
+        QHostInfo::lookupHost("localhost", &contextObject, [](const QHostInfo){
+            QFAIL("This should never be called!");
+        });
+    }
+    QTestEventLoop::instance().enterLoop(3);
+}
+
 void tst_QHostInfo::lookupConnectToLambda_data()
 {
     lookupIPv4_data();
@@ -708,6 +720,7 @@ void tst_QHostInfo::cache()
 
 void tst_QHostInfo::resultsReady(const QHostInfo &hi)
 {
+    QVERIFY(QThread::currentThread() == thread());
     lookupDone = true;
     lookupResults = hi;
     lookupsDoneCounter++;