Clear pending responses in reader buffer on disconnect.

When using the phpiredis-based connection backends, failed pipelines led to spurious responses returned after reconnecting to Redis because the underlying reader was not properly reset by discarding buffered replies after disconnecting. Fixes a couple of issues reported in #363.
predis · Sep 20, 2020 · 5ae4ac6 · 5ae4ac6
1 parent 521ac8f
commit 5ae4ac6
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,12 @@
+v1.1.7 (2020-xx-xx)
+================================================================================
+
+- __FIX__: with the phpiredis-based connection backends, failed pipelines led to
+spurious responses returned after reconnecting to Redis because the underlying
+reader was not properly reset by discarding buffered replies after disconnecting
+(ISSUE #363).
+
+
 v1.1.6 (2020-09-11)
 ================================================================================
 

diff --git a/src/Connection/PhpiredisSocketConnection.php b/src/Connection/PhpiredisSocketConnection.php
@@ -66,9 +66,9 @@ public function __construct(ParametersInterface $parameters)
      */
     public function __destruct()
     {
-        phpiredis_reader_destroy($this->reader);
-
         parent::__destruct();
+
+        phpiredis_reader_destroy($this->reader);
     }
 
     /**
@@ -342,7 +342,9 @@ public function connect()
     public function disconnect()
     {
         if ($this->isConnected()) {
+            phpiredis_reader_reset($this->reader);
             socket_close($this->getResource());
+
             parent::disconnect();
         }
     }

diff --git a/src/Connection/PhpiredisStreamConnection.php b/src/Connection/PhpiredisStreamConnection.php
@@ -67,9 +67,19 @@ public function __construct(ParametersInterface $parameters)
      */
     public function __destruct()
     {
+        parent::__destruct();
+
         phpiredis_reader_destroy($this->reader);
+    }
 
-        parent::__destruct();
+    /**
+     * {@inheritdoc}
+     */
+    public function disconnect()
+    {
+        phpiredis_reader_reset($this->reader);
+
+        parent::disconnect();
     }
 
     /**

diff --git a/tests/Predis/Connection/PhpiredisSocketConnectionTest.php b/tests/Predis/Connection/PhpiredisSocketConnectionTest.php
@@ -76,6 +76,28 @@ public function testThrowsExceptionOnInitializationCommandFailure()
     // ---- INTEGRATION TESTS --------------------------------------------- //
     // ******************************************************************** //
 
+    /**
+     * @group connected
+     */
+    public function testClearsPendingResponsesInReaderBufferOnDisconnect()
+    {
+        $profile = $this->getCurrentProfile();
+        $connection = $this->createConnection();
+
+        $cmdECHO1 = $profile->createCommand('echo', array('BEFORE DISCONNECT 1'));
+        $cmdECHO2 = $profile->createCommand('echo', array('BEFORE DISCONNECT 2'));
+        $cmdECHO3 = $profile->createCommand('echo', array('AFTER DISCONNECT 1'));
+
+        $connection->writeRequest($cmdECHO1);
+        $connection->writeRequest($cmdECHO2);
+        $connection->readResponse($cmdECHO1);
+        $connection->disconnect();
+
+        $response = $connection->executeCommand($cmdECHO3);
+
+        $this->assertSame('AFTER DISCONNECT 1', $response);
+    }
+
     /**
      * @group connected
      * @expectedException \Predis\Connection\ConnectionException

diff --git a/tests/Predis/Connection/PhpiredisStreamConnectionTest.php b/tests/Predis/Connection/PhpiredisStreamConnectionTest.php
@@ -76,6 +76,28 @@ public function testThrowsExceptionOnInitializationCommandFailure()
     // ---- INTEGRATION TESTS --------------------------------------------- //
     // ******************************************************************** //
 
+    /**
+     * @group connected
+     */
+    public function testClearsPendingResponsesInReaderBufferOnDisconnect()
+    {
+        $profile = $this->getCurrentProfile();
+        $connection = $this->createConnection();
+
+        $cmdECHO1 = $profile->createCommand('echo', array('BEFORE DISCONNECT 1'));
+        $cmdECHO2 = $profile->createCommand('echo', array('BEFORE DISCONNECT 2'));
+        $cmdECHO3 = $profile->createCommand('echo', array('AFTER DISCONNECT 1'));
+
+        $connection->writeRequest($cmdECHO1);
+        $connection->writeRequest($cmdECHO2);
+        $connection->readResponse($cmdECHO1);
+        $connection->disconnect();
+
+        $response = $connection->executeCommand($cmdECHO3);
+
+        $this->assertSame('AFTER DISCONNECT 1', $response);
+    }
+
     /**
      * @group connected
      * @group slow