QPacketProtocol: Add proper error handling

The device is not guaranteed to write or read all the data we ask for in
one go. Retry and check for read or write errors.

Change-Id: I640bdaf4d9eb7b900c8f7145bd42eb5d667a9cad
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

3 files changed, 57 insertions, 20 deletions

diff --git a/src/plugins/qmltooling/packetprotocol/qpacketprotocol.cpp b/src/plugins/qmltooling/packetprotocol/qpacketprotocol.cpp
index 5460d617dd..791c09810e 100644
--- a/src/plugins/qmltooling/packetprotocol/qpacketprotocol.cpp
+++ b/src/plugins/qmltooling/packetprotocol/qpacketprotocol.cpp
@@ -106,6 +106,9 @@ class QPacketProtocolPrivate : public QObjectPrivate
 public:
     QPacketProtocolPrivate(QIODevice *dev);
 
+    bool writeToDevice(const char *bytes, qint64 size);
+    bool readFromDevice(char *buffer, qint64 size);
+
     QList<qint64> sendingPackets;
     QList<QByteArray> packets;
     QByteArray inProgress;
@@ -140,15 +143,14 @@ void QPacketProtocol::send(const QByteArray &data)
 
     if (data.isEmpty())
         return; // We don't send empty packets
-    qint64 sendSize = data.size() + sizeof(qint32);
+    const qint32 sendSize = data.size() + static_cast<qint32>(sizeof(qint32));
 
     d->sendingPackets.append(sendSize);
-    qint32 sendSize32 = sendSize;
-    qint64 writeBytes = d->dev->write((char *)&sendSize32, sizeof(qint32));
-    Q_UNUSED(writeBytes);
-    Q_ASSERT(writeBytes == sizeof(qint32));
-    writeBytes = d->dev->write(data);
-    Q_ASSERT(writeBytes == data.size());
+    if (!d->writeToDevice((const char *)&sendSize, sizeof(qint32))
+            || !d->writeToDevice(data.data(), data.size())) {
+        emit error();
+        return;
+    }
 }
 
 /*!
@@ -234,13 +236,14 @@ void QPacketProtocol::readyToRead()
         // Need to get trailing data
         if (-1 == d->inProgressSize) {
             // We need a size header of sizeof(qint32)
-            if (sizeof(qint32) > (uint)d->dev->bytesAvailable())
+            if (static_cast<qint64>(sizeof(qint32)) > d->dev->bytesAvailable())
                 return;
 
             // Read size header
-            int read = d->dev->read((char *)&d->inProgressSize, sizeof(qint32));
-            Q_ASSERT(read == sizeof(qint32));
-            Q_UNUSED(read);
+            if (!d->readFromDevice((char *)&d->inProgressSize, sizeof(qint32))) {
+                emit error();
+                return;
+            }
 
             // Check sizing constraints
             if (d->inProgressSize > MAX_PACKET_SIZE) {
@@ -248,14 +251,24 @@ void QPacketProtocol::readyToRead()
                 disconnect(d->dev, &QIODevice::aboutToClose, this, &QPacketProtocol::aboutToClose);
                 disconnect(d->dev, &QIODevice::bytesWritten, this, &QPacketProtocol::bytesWritten);
                 d->dev = nullptr;
-                emit invalidPacket();
+                emit error();
                 return;
             }
 
             d->inProgressSize -= sizeof(qint32);
         } else {
-            d->inProgress.append(d->dev->read(d->inProgressSize - d->inProgress.size()));
 
+            const int bytesToRead = static_cast<int>(
+                        qMin(d->dev->bytesAvailable(),
+                             static_cast<qint64>(d->inProgressSize - d->inProgress.size())));
+
+            QByteArray toRead(bytesToRead, Qt::Uninitialized);
+            if (!d->readFromDevice(toRead.data(), toRead.length())) {
+                emit error();
+                return;
+            }
+
+            d->inProgress.append(toRead);
             if (d->inProgressSize == d->inProgress.size()) {
                 // Packet has arrived!
                 d->packets.append(d->inProgress);
@@ -275,6 +288,30 @@ QPacketProtocolPrivate::QPacketProtocolPrivate(QIODevice *dev) :
 {
 }
 
+bool QPacketProtocolPrivate::writeToDevice(const char *bytes, qint64 size)
+{
+    qint64 totalWritten = 0;
+    while (totalWritten < size) {
+        const qint64 chunkSize = dev->write(bytes + totalWritten, size - totalWritten);
+        if (chunkSize < 0)
+            return false;
+        totalWritten += chunkSize;
+    }
+    return totalWritten == size;
+}
+
+bool QPacketProtocolPrivate::readFromDevice(char *buffer, qint64 size)
+{
+    qint64 totalRead = 0;
+    while (totalRead < size) {
+        const qint64 chunkSize = dev->read(buffer + totalRead, size - totalRead);
+        if (chunkSize < 0)
+            return false;
+        totalRead += chunkSize;
+    }
+    return totalRead == size;
+}
+
 /*!
   \fn void QPacketProtocol::readyRead()
 
diff --git a/src/plugins/qmltooling/packetprotocol/qpacketprotocol_p.h b/src/plugins/qmltooling/packetprotocol/qpacketprotocol_p.h
index 35edb568aa..a478fc9996 100644
--- a/src/plugins/qmltooling/packetprotocol/qpacketprotocol_p.h
+++ b/src/plugins/qmltooling/packetprotocol/qpacketprotocol_p.h
@@ -72,7 +72,7 @@ public:
 
 Q_SIGNALS:
     void readyRead();
-    void invalidPacket();
+    void error();
 
 private:
     void aboutToClose();
diff --git a/src/plugins/qmltooling/qmldbg_server/qqmldebugserver.cpp b/src/plugins/qmltooling/qmldbg_server/qqmldebugserver.cpp
index 0a468c0b84..27b310a0ae 100644
--- a/src/plugins/qmltooling/qmldbg_server/qqmldebugserver.cpp
+++ b/src/plugins/qmltooling/qmldbg_server/qqmldebugserver.cpp
@@ -176,7 +176,7 @@ private:
     void changeServiceState(const QString &serviceName, QQmlDebugService::State state);
     void removeThread();
     void receiveMessage();
-    void invalidPacket();
+    void protocolError();
 
     QQmlDebugServerConnection *m_connection;
     QHash<QString, QQmlDebugService *> m_plugins;
@@ -521,7 +521,7 @@ void QQmlDebugServerImpl::receiveMessage()
 
         } else {
             qWarning("QML Debugger: Invalid control message %d.", op);
-            invalidPacket();
+            protocolError();
             return;
         }
 
@@ -736,16 +736,16 @@ void QQmlDebugServerImpl::setDevice(QIODevice *socket)
     m_protocol = new QPacketProtocol(socket, this);
     QObject::connect(m_protocol, &QPacketProtocol::readyRead,
                      this, &QQmlDebugServerImpl::receiveMessage);
-    QObject::connect(m_protocol, &QPacketProtocol::invalidPacket,
-                     this, &QQmlDebugServerImpl::invalidPacket);
+    QObject::connect(m_protocol, &QPacketProtocol::error,
+                     this, &QQmlDebugServerImpl::protocolError);
 
     if (blockingMode())
         m_protocol->waitForReadyRead(-1);
 }
 
-void QQmlDebugServerImpl::invalidPacket()
+void QQmlDebugServerImpl::protocolError()
 {
-    qWarning("QML Debugger: Received a corrupted packet! Giving up ...");
+    qWarning("QML Debugger: A protocol error has occurred! Giving up ...");
     m_connection->disconnect();
     // protocol might still be processing packages at this point
     m_protocol->deleteLater();