[Client] Treat a busy single-instance primary as alive instead of stealing its socket

cockatrice/src/single_instance_manager.cpp

@@ -28,9 +28,15 @@ bool SingleInstanceManager::tryRun(const QStringList &filesToSend)
     }
     serverName = QStringLiteral("CockatriceSingleInstance-%1").arg(userName);
 
-    // Hand off to an already-running primary instance if one exists.
-    if (forwardToPrimary(filesToSend)) {
+    // Hand off to an already-running primary instance if one exists. Never steal
+    // the socket of a busy primary: it is alive and will act on the payload.
+    switch (forwardToPrimary(filesToSend)) {
+        case ForwardResult::Delivered:
             return false;
+        case ForwardResult::PrimaryBusy:
+            return false;
+        case ForwardResult::NoPrimary:
+            break;
     }
 
     // No primary instance is currently reachable, so become the primary.
@@ -43,12 +49,18 @@ bool SingleInstanceManager::tryRun(const QStringList &filesToSend)
 
     // Another instance may have started while we were probing; hand off to it
     // instead of stealing its socket.
-    if (forwardToPrimary(filesToSend)) {
+    switch (forwardToPrimary(filesToSend)) {
+        case ForwardResult::Delivered:
             return false;
+        case ForwardResult::PrimaryBusy:
+            return false;
+        case ForwardResult::NoPrimary:
+            break;
     }
 
-    // The socket is stale (left over by a crashed instance): remove it and
-    // retry. If that still fails, another instance just took the name.
+    // The socket is stale (left over by a crashed instance), so no primary is
+    // holding it: remove it and retry. If that still fails, another instance
+    // just took the name.
     QLocalServer::removeServer(serverName);
     if (server->listen(serverName)) {
         return true;
@@ -58,12 +70,12 @@ bool SingleInstanceManager::tryRun(const QStringList &filesToSend)
     return false;
 }
 
-bool SingleInstanceManager::forwardToPrimary(const QStringList &filesToSend)
+SingleInstanceManager::ForwardResult SingleInstanceManager::forwardToPrimary(const QStringList &filesToSend)
 {
     QLocalSocket socket;
     socket.connectToServer(serverName);
     if (!socket.waitForConnected(200)) {
-        return false;
+        return ForwardResult::NoPrimary;
     }
 
     // Serialize payload with length prefix
@@ -81,13 +93,14 @@ bool SingleInstanceManager::forwardToPrimary(const QStringList &filesToSend)
     socket.waitForBytesWritten(1000);
 
     // Only report a successful hand-off once the primary has acknowledged that
-    // it actually read the payload. A socket that connects but never answers
-    // belongs to a process that is dying, so the caller must not treat this as
-    // a hand-off (otherwise it would exit without anyone handling the files).
-    if (!socket.waitForReadyRead(1000)) {
-        return false;
+    // it actually read the payload. A socket that connects but is still working
+    // on an earlier payload is alive but busy, not dead: give it more room
+    // before giving up, so a slow handler does not make a live primary look
+    // dead (which would lead to stealing its socket).
+    if (!socket.waitForReadyRead(1000) && !socket.waitForReadyRead(4000)) {
+        return ForwardResult::PrimaryBusy;
     }
-    return socket.readAll() == ACK_MESSAGE;
+    return socket.readAll() == ACK_MESSAGE ? ForwardResult::Delivered : ForwardResult::PrimaryBusy;
 }
 
 void SingleInstanceManager::handleNewConnection()

cockatrice/src/single_instance_manager.h

@@ -23,7 +23,14 @@ private slots:
     void handleNewConnection();
 
 private:
-    bool forwardToPrimary(const QStringList &filesToSend);
+    enum class ForwardResult
+    {
+        Delivered,  // a live primary acknowledged the payload
+        NoPrimary,  // no connectable primary socket exists
+        PrimaryBusy // a primary exists but did not acknowledge in time
+    };
+
+    ForwardResult forwardToPrimary(const QStringList &filesToSend);
 
     QString serverName;
     QLocalServer *server = nullptr;