Fix request memory leak and clean up on disconnected, caused by core update

snapd/devicepluginsnapd.cpp

@@ -149,7 +149,7 @@ DeviceManager::DeviceError DevicePluginSnapd::executeAction(Device *device, cons
         }
 
         if (!m_snapdControl->connected()) {
-            qCDebug(dcSnapd()) << "Snapd controller not connected to to backend.";
+            qCDebug(dcSnapd()) << "Snapd controller not connected to the backend.";
             return DeviceManager::DeviceErrorHardwareFailure;
         }
 

snapd/snapdconnection.cpp

@@ -22,21 +22,9 @@ SnapdReply *SnapdConnection::get(const QString &path)
     reply->setRequestMethod("GET");
     reply->setRequestRawMessage(createRequestHeader("GET", path));
 
-    // Check if currently a reply is running
+    // Enqueue the new reply
-    if (m_currentReply) {
+    m_replyQueue.enqueue(reply);
-        m_replyQueue.enqueue(reply);
+    sendNextRequest();
-    } else {
-        // Send request
-        m_currentReply = reply;
-        if (m_debug)
-            qCDebug(dcSnapd()) << "-->" << reply->requestMethod() << reply->requestPath();
-
-        if (write(reply->requestRawMessage()) <= 0) {
-            m_currentReply = nullptr;
-            reply->setFinished(false);
-            sendNextRequest();
-        }
-    }
 
     // Note: the caller owns the object now
     return reply;
@@ -50,21 +38,9 @@ SnapdReply *SnapdConnection::post(const QString &path, const QByteArray &payload
     QByteArray header = createRequestHeader("POST", path, payload);
     reply->setRequestRawMessage(header.append(payload));
 
-    // Check if currently a reply is running
+    // Enqueue the new reply
-    if (m_currentReply) {
+    m_replyQueue.enqueue(reply);
-        m_replyQueue.enqueue(reply);
+    sendNextRequest();
-    } else {
-        // Send request
-        m_currentReply = reply;
-        if (m_debug)
-            qCDebug(dcSnapd()) << "-->" << reply->requestMethod() << reply->requestPath() << payload;
-
-        if (write(reply->requestRawMessage()) <= 0) {
-            m_currentReply = nullptr;
-            reply->setFinished(false);
-            sendNextRequest();
-        }
-    }
 
     // Note: the caller owns the object now
     return reply;
@@ -80,9 +56,22 @@ void SnapdConnection::setConnected(const bool &connected)
     if (m_connected == connected)
         return;
 
-    qCDebug(dcSnapd()) << "Connected";
-
     m_connected = connected;
+
+    // Clean up replies of disconnected
+    if (!m_connected) {
+        foreach (SnapdReply *reply, m_replyQueue) {
+            reply->setFinished(false);
+        }
+
+        if (m_currentReply) {
+            m_currentReply->setFinished(false);
+            m_currentReply = nullptr;
+        }
+
+        m_replyQueue.clear();
+    }
+
     emit connectedChanged(m_connected);
 }
 
@@ -210,26 +199,35 @@ void SnapdConnection::processData()
     m_currentReply->setHeader(parsedHeader);
     m_currentReply->setDataMap(jsonDoc.toVariant().toMap());
     m_currentReply->setFinished();
-    m_currentReply = nullptr;
-
-    sendNextRequest();
 
     // Current data stream finished, reset for new messages
     m_payload.clear();
     m_header.clear();
     m_chuncked = false;
+
+    // Ready for next reply
+    m_currentReply = nullptr;
+    sendNextRequest();
 }
 
 void SnapdConnection::sendNextRequest()
 {
+    // Check if nothing else to do
     if (m_replyQueue.isEmpty())
         return;
 
+    // Check a reply is currently pending
+    if (m_currentReply)
+        return;
+
+    // Dequeue and send next reply
     SnapdReply *reply = m_replyQueue.dequeue();
     m_currentReply = reply;
+
     if (m_debug)
         qCDebug(dcSnapd()) << "-->" << reply->requestMethod() << reply->requestPath();
 
+    // If write failes, the reply is finished invalid and the owner has to delete it
     if (write(reply->requestRawMessage()) < 0) {
         m_currentReply->setFinished(false);
         m_currentReply = nullptr;

snapd/snapdcontrol.cpp

@@ -131,7 +131,7 @@ void SnapdControl::processChange(const QVariantMap &changeMap)
 
     qCDebug(dcSnapd()) << changeStatus << changeKind << changeSummary;
 
-    // Add this change if not alreade finishished or added
+    // Add this change if not already finishished or added
     if (!m_watchingChanges.contains(changeId) && !changeReady)
         m_watchingChanges.append(changeId);
 
@@ -140,6 +140,7 @@ void SnapdControl::processChange(const QVariantMap &changeMap)
         device()->setStateValue(statusStateTypeId, changeSummary);
     }
 
+    // If this change is on ready, we can remove it from our watch list
     if (changeReady) {
         qCDebug(dcSnapd()).noquote() << changeKind << (changeReady ? "finished." : "running.") << changeSummary;
         m_watchingChanges.removeAll(changeId);
@@ -213,10 +214,16 @@ void SnapdControl::onLoadRunningChangesFinished()
         processChange(changeVariant.toMap());
     }
 
-    if (reply->dataMap().value("result").toList().isEmpty() && m_watchingChanges.isEmpty()) {
+    // Check if there are still changes around
+    if (reply->dataMap().value("result").toList().isEmpty()) {
+        // If there are no running changes, we can forget old ones
+        m_watchingChanges.clear();
+
+        // Update not running any more
         device()->setStateValue(updateRunningStateTypeId, false);
         device()->setStateValue(statusStateTypeId, "-");
     } else {
+        // Update running
         device()->setStateValue(updateRunningStateTypeId, true);
     }
 
@@ -236,7 +243,7 @@ void SnapdControl::onLoadChangeFinished()
 
     if (m_watchingChanges.isEmpty()) {
         device()->setStateValue(updateRunningStateTypeId, false);
-
+        device()->setStateValue(statusStateTypeId, "-");
     }
 
     reply->deleteLater();