Fix deadlock when issueing a blocking command on a connection that ha…

…d a failed blocking command before
bzikarsky · Jan 16, 2018 · 48123ef · 48123ef
1 parent 08f442d
commit 48123ef
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 11 deletions.
diff --git a/src/Protocol/Participant.php b/src/Protocol/Participant.php
@@ -186,18 +186,15 @@ private function sendDeferred(CommandInterface $command, Deferred $deferred, Pro
         //  - install an error-handler to communicate the failure
         if ($lock) {
             $this->sendLocked = true;
-            $lock->then(
-                function () {
-                    $this->sendLocked = false;
-                    if (!empty($this->sendQueue)) {
-                        list($command, $deferred, $lock) = array_shift($this->sendQueue);
-                        $this->sendDeferred($command, $deferred, $lock);
-                    }
-                },
-                function () {
-                    throw new ProtocolException("Blocking operation failed. Protocol is in invalid state");
+            $doUnlock = function () {
+                $this->sendLocked = false;
+                if (!empty($this->sendQueue)) {
+                    list($command, $deferred, $lock) = array_shift($this->sendQueue);
+                    $this->sendDeferred($command, $deferred, $lock);
                 }
-            );
+            };
+            // Also unlock if previous command failed, otherwise there is a deadlock when issueing a blocked command after a failure
+            $lock->then($doUnlock, $doUnlock);
         }
 
         // write the command

diff --git a/tests/ClientTest.php b/tests/ClientTest.php
@@ -376,6 +376,28 @@ public function testSubmitWithLostConnectionAfterWrite()
         $this->assertInstanceOf(\Zikarsky\React\Gearman\ConnectionLostException::class, $exception);
     }
 
+    public function testSubmitAfterLostConnection()
+    {
+        $this->connection->expects($this->exactly(2))->method('send')->will($this->returnValue(new FulfilledPromise()));
+
+        $this->client->submit('func', 'data');
+        $this->connection->emit('close');
+
+        // Second action must also produce a send call on the connection
+        $this->client->submit('func', 'data');
+    }
+
+    public function testSubmitAfterLostConnection2()
+    {
+        $this->connection->expects($this->exactly(2))->method('send')->will($this->returnValue(new FulfilledPromise()));
+
+        $this->client->submit('func', 'data');
+        // Second action must also produce a send call on the connection
+        $this->client->submit('func', 'data');
+
+        $this->connection->emit('close');
+    }
+
     public function testSetOption()
     {
         $confirmed = false;