RemoteObject: require server reply for all client commands

For classes supporting the remote-client-server protocol through
inheritance of the QInstaller::RemoteObject class: when calling
functions of return type void, the client process did not block and
wait for the server process to finish execution of the corresponding
function, only checking that the packet containing the function
command and arguments was written to the socket.

The server-side connection thread operates in a loop that tries to
read a packet from the socket in each iteration, and process the
contained command, with the condition that the socket is still in
connected state. If the function calls requested by the commands take
long time to execute and we destruct the client-side object,
disconnecting the socket, the server thread could break out of the
loop before handling all pending packets.

This hasn't been a problem for non-void functions because the server
will send a return value reply and the client would block until the
sent packet was available. Fix by refactoring the server connection
code to send a default reply for commands not requiring a real return
value, and blocking the client until the reply can be read.

Task-number: QTIFW-999
Change-Id: Ie1d0e118e6830a1049a2ad50cc80b54c967fcde6
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Katja Marttila <katja.marttila@qt.io>

1 files changed, 44 insertions, 29 deletions

diff --git a/src/libs/installer/remoteobject.h b/src/libs/installer/remoteobject.h
index ecd86c984..4dce2a218 100644
--- a/src/libs/installer/remoteobject.h
+++ b/src/libs/installer/remoteobject.h
@@ -1,6 +1,6 @@
 /**************************************************************************
 **
-** Copyright (C) 2017 The Qt Company Ltd.
+** Copyright (C) 2022 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the Qt Installer Framework.
@@ -55,59 +55,39 @@ public:
     template<typename T1, typename T2>
     void callRemoteMethod(const QString &name, const T1 &arg, const T2 &arg2)
     {
-        writeData(name, arg, arg2, dummy);
+        const QString reply = sendReceivePacket<QString>(name, arg, arg2, dummy);
+        Q_ASSERT(reply == QLatin1String(Protocol::DefaultReply));
     }
 
     template<typename T1, typename T2, typename T3>
     void callRemoteMethod(const QString &name, const T1 &arg, const T2 &arg2, const T3 & arg3)
     {
-        writeData(name, arg, arg2, arg3);
+        const QString reply = sendReceivePacket<QString>(name, arg, arg2, arg3);
+        Q_ASSERT(reply == QLatin1String(Protocol::DefaultReply));
     }
 
     template<typename T>
     T callRemoteMethod(const QString &name) const
     {
-        return callRemoteMethod<T>(name, dummy, dummy, dummy);
+        return sendReceivePacket<T>(name, dummy, dummy, dummy);
     }
 
     template<typename T, typename T1>
     T callRemoteMethod(const QString &name, const T1 &arg) const
     {
-        return callRemoteMethod<T>(name, arg, dummy, dummy);
+        return sendReceivePacket<T>(name, arg, dummy, dummy);
     }
 
     template<typename T, typename T1, typename T2>
     T callRemoteMethod(const QString &name, const T1 & arg, const T2 &arg2) const
     {
-        return callRemoteMethod<T>(name, arg, arg2, dummy);
+        return sendReceivePacket<T>(name, arg, arg2, dummy);
     }
 
     template<typename T, typename T1, typename T2, typename T3>
     T callRemoteMethod(const QString &name, const T1 &arg, const T2 &arg2, const T3 &arg3) const
     {
-        writeData(name, arg, arg2, arg3);
-        while (m_socket->bytesToWrite())
-            m_socket->waitForBytesWritten();
-
-        QByteArray command;
-        QByteArray data;
-        while (!receivePacket(m_socket, &command, &data)) {
-            if (!m_socket->waitForReadyRead(-1)) {
-                throw Error(tr("Cannot read all data after sending command: %1. "
-                    "Bytes expected: %2, Bytes received: %3. Error: %4").arg(name).arg(0)
-                    .arg(m_socket->bytesAvailable()).arg(m_socket->errorString()));
-            }
-        }
-
-        Q_ASSERT(command == Protocol::Reply);
-
-        QDataStream stream(&data, QIODevice::ReadOnly);
-
-        T result;
-        stream >> result;
-        Q_ASSERT(stream.status() == QDataStream::Ok);
-        Q_ASSERT(stream.atEnd());
-        return result;
+        return sendReceivePacket<T>(name, arg, arg2, arg3);
     }
 
 protected:
@@ -133,6 +113,17 @@ private:
         return false;
     }
 
+    template<typename T, typename T1, typename T2, typename T3>
+    T sendReceivePacket(const QString &name, const T1 &arg, const T2 &arg2, const T3 &arg3) const
+    {
+        writeData(name, arg, arg2, arg3);
+        while (m_socket->bytesToWrite())
+            m_socket->waitForBytesWritten();
+
+        return readData<T>(name);
+    }
+
+
     template<typename T1, typename T2, typename T3>
     void writeData(const QString &name, const T1 &arg, const T2 &arg2, const T3 &arg3) const
     {
@@ -150,6 +141,30 @@ private:
         m_socket->flush();
     }
 
+    template<typename T>
+    T readData(const QString &name) const
+    {
+        QByteArray command;
+        QByteArray data;
+        while (!receivePacket(m_socket, &command, &data)) {
+            if (!m_socket->waitForReadyRead(-1)) {
+                throw Error(tr("Cannot read all data after sending command: %1. "
+                    "Bytes expected: %2, Bytes received: %3. Error: %4").arg(name).arg(0)
+                    .arg(m_socket->bytesAvailable()).arg(m_socket->errorString()));
+            }
+        }
+
+        Q_ASSERT(command == Protocol::Reply);
+
+        QDataStream stream(&data, QIODevice::ReadOnly);
+
+        T result;
+        stream >> result;
+        Q_ASSERT(stream.status() == QDataStream::Ok);
+        Q_ASSERT(stream.atEnd());
+        return result;
+    }
+
 private:
     QString m_type;
     QLocalSocket *m_socket;