Update CHANGELOG and bump VERSION.

The following code, despite not being something you would do in real
world scenarios, eventually ended up generating an exception for "Too
many open files".

  for ($i = 0; $i < 10000; $i++) {
    $client = new Predis\Client();
    $client->connect();
  }

The reason was that the destructor for the connection was not invoked
by PHP as expected when the client instance went out of scope, so the
underlying stream resource were not being properly released.
Even without an actual "connect()" operation, the memory usage of PHP
kept growing until exhausting the configured value of max memory.

The source of the issue is related to the use of closures as handlers
for the phpiredis reader resource, to be more precise it seems to be
some kind of circular reference memory leak. Apparently PHP does not
like the fact that a closure, automatically bound to "$this" starting
with PHP 5.4, is stored in the reader resource which in turn is kept
referenced by "$this". This ends up the refcount not going down thus
the GC does not collect those connection objects going out of scope.
This is confirmed by the fact that this issue is not triggered when
using PHP 5.3 where the ZE does not automatically bind closures to
"$this", unless you capture "$this" with the "use()" directive (and
the usual "$that = $this" trick).

Using a static assignement instead of simply returning new closures
in "getStatusHandler()" and "getErrorHandler()" is kind of an hack
that seems to be working fine, the added value of this solution is
that we do not have to break the internal API of the three connection
backends based on ext-phpiredis.

This commit fixes #345.

CHANGELOG.md

@@ -1,3 +1,22 @@
+v1.1.1 (2016-06-16)
+================================================================================
+
+- __FIX__: `password` and `database` from the global `parameters` client option
+  were still being applied to sentinels connections making them fail (sentinels
+  do not understand the `AUTH` and `SELECT` commands) (PR #346).
+
+- __FIX__: when a sentinel instance reports no sentinel for a service, invoking
+  `connect()` on the redis-sentinel connection backend should fall back to the
+  master connection instead of failing (ISSUE #342).
+
+- __FIX__: the two connection backends based on ext-phpiredis has some kind of
+  issues with the GC and the internal use of closures as reader callbacks that
+  prevented connections going out of scope from being properly collected and the
+  underlying stream or socket resources from being closed and freed. This should
+  not have had any actual effect in real-world scenarios due to the lifecycle of
+  PHP scripts, but we fixed it anyway (ISSUE #345).
+
+
 v1.1.0 (2016-06-02)
 ================================================================================
 

VERSION

@@ -1 +1 @@
-1.1.0
+1.1.1

package.ini

@@ -10,7 +10,7 @@ name        = "Predis"
 desc        = "Flexible and feature-complete Redis client for PHP and HHVM"
 homepage    = "http://github.com/nrk/predis"
 license     = "MIT"
-version     = "1.1.0"
+version     = "1.1.1"
 stability   = "stable"
 channel     = "pear.nrk.io"
 

src/Client.php

@@ -40,7 +40,7 @@ use Predis\Transaction\MultiExec as MultiExecTransaction;
  */
 class Client implements ClientInterface, \IteratorAggregate
 {
-    const VERSION = '1.1.0';
+    const VERSION = '1.1.1';
 
     protected $connection;
     protected $options;
@@ -278,7 +278,7 @@ class Client implements ClientInterface, \IteratorAggregate
      * applying any prefix to keys or throwing exceptions on Redis errors even
      * regardless of client options.
      *
-     * It is possibile to indentify Redis error responses from normal responses
+     * It is possible to identify Redis error responses from normal responses
      * using the second optional argument which is populated by reference.
      *
      * @param array $arguments Command arguments as defined by the command signature.
@@ -481,7 +481,7 @@ class Client implements ClientInterface, \IteratorAggregate
     }
 
     /**
-     * Creates a new publis/subscribe context and returns it, or starts its loop
+     * Creates a new publish/subscribe context and returns it, or starts its loop
      * inside the optionally provided callable object.
      *
      * @param mixed ... Array of options, a callable for execution, or both.

src/ClientContextInterface.php

@@ -38,7 +38,7 @@ use Predis\Command\CommandInterface;
  * @method $this append($key, $value)
  * @method $this bitcount($key, $start = null, $end = null)
  * @method $this bitop($operation, $destkey, $key)
- * @method $this bitfield($key, ...)
+ * @method $this bitfield($key, $subcommand, ...$subcommandArg)
  * @method $this decr($key)
  * @method $this decrby($key, $decrement)
  * @method $this get($key)

src/ClientInterface.php

@@ -46,7 +46,7 @@ use Predis\Profile\ProfileInterface;
  * @method int    append($key, $value)
  * @method int    bitcount($key, $start = null, $end = null)
  * @method int    bitop($operation, $destkey, $key)
- * @method array  bitfield($key, ...)
+ * @method array  bitfield($key, $subcommand, ...$subcommandArg)
  * @method int    decr($key)
  * @method int    decrby($key, $decrement)
  * @method string get($key)

src/Connection/Aggregate/SentinelReplication.php

@@ -239,9 +239,10 @@ class SentinelReplication implements ReplicationInterface
         }
 
         if (is_array($parameters)) {
-            // We unset "password" and "database" from user-supplied parameters
-            // as they are not needed when connecting to sentinels.
-            unset($parameters['database'], $parameters['password']);
+            // We explicitly set "database" and "password" to null,
+            // so that no AUTH and SELECT command is send to the sentinels.
+            $parameters['database'] = null;
+            $parameters['password'] = null;
 
             if (!isset($parameters['timeout'])) {
                 $parameters['timeout'] = $this->sentinelTimeout;
@@ -617,7 +618,9 @@ class SentinelReplication implements ReplicationInterface
     public function connect()
     {
         if (!$this->current) {
-            $this->current = $this->pickSlave();
+            if (!$this->current = $this->pickSlave()) {
+                $this->current = $this->getMaster();
+            }
         }
 
         $this->current->connect();

src/Connection/PhpiredisSocketConnection.php

@@ -143,11 +143,17 @@ class PhpiredisSocketConnection extends AbstractConnection
      *
      * @return \Closure
      */
-    private function getStatusHandler()
+    protected function getStatusHandler()
     {
-        return function ($payload) {
-            return StatusResponse::get($payload);
-        };
+        static $statusHandler;
+
+        if (!$statusHandler) {
+            $statusHandler = function ($payload) {
+                return StatusResponse::get($payload);
+            };
+        }
+
+        return $statusHandler;
     }
 
     /**
@@ -157,9 +163,15 @@ class PhpiredisSocketConnection extends AbstractConnection
      */
     protected function getErrorHandler()
     {
-        return function ($payload) {
-            return new ErrorResponse($payload);
-        };
+        static $errorHandler;
+
+        if (!$errorHandler) {
+            $errorHandler = function ($errorMessage) {
+                return new ErrorResponse($errorMessage);
+            };
+        }
+
+        return $errorHandler;
     }
 
     /**

src/Connection/PhpiredisStreamConnection.php

@@ -160,9 +160,15 @@ class PhpiredisStreamConnection extends StreamConnection
      */
     protected function getStatusHandler()
     {
-        return function ($payload) {
-            return StatusResponse::get($payload);
-        };
+        static $statusHandler;
+
+        if (!$statusHandler) {
+            $statusHandler = function ($payload) {
+                return StatusResponse::get($payload);
+            };
+        }
+
+        return $statusHandler;
     }
 
     /**
@@ -172,9 +178,15 @@ class PhpiredisStreamConnection extends StreamConnection
      */
     protected function getErrorHandler()
     {
-        return function ($errorMessage) {
-            return new ErrorResponse($errorMessage);
-        };
+        static $errorHandler;
+
+        if (!$errorHandler) {
+            $errorHandler = function ($errorMessage) {
+                return new ErrorResponse($errorMessage);
+            };
+        }
+
+        return $errorHandler;
     }
 
     /**

src/Connection/WebdisConnection.php

@@ -163,9 +163,15 @@ class WebdisConnection implements NodeConnectionInterface
      */
     protected function getStatusHandler()
     {
-        return function ($payload) {
-            return StatusResponse::get($payload);
-        };
+        static $statusHandler;
+
+        if (!$statusHandler) {
+            $statusHandler = function ($payload) {
+                return StatusResponse::get($payload);
+            };
+        }
+
+        return $statusHandler;
     }
 
     /**
@@ -175,9 +181,15 @@ class WebdisConnection implements NodeConnectionInterface
      */
     protected function getErrorHandler()
     {
-        return function ($payload) {
-            return new ErrorResponse($payload);
-        };
+        static $errorHandler;
+
+        if (!$errorHandler) {
+            $errorHandler = function ($errorMessage) {
+                return new ErrorResponse($errorMessage);
+            };
+        }
+
+        return $errorHandler;
     }
 
     /**

tests/Predis/Cluster/Hash/CRC16Test.php

@@ -0,0 +1,39 @@
+<?php
+
+/*
+ * This file is part of the Predis package.
+ *
+ * (c) Daniele Alessandri <suppakilla@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Predis\Cluster\Hash;
+
+use PredisTestCase;
+
+/**
+ *
+ */
+class CRC16Test extends PredisTestCase
+{
+    /**
+     * @group disconnected
+     */
+    public function testHashGeneration()
+    {
+        $crc16 = new CRC16();
+
+        $this->assertSame(58359, $crc16->hash('key:000'));
+        $this->assertSame(62422, $crc16->hash('key:001'));
+        $this->assertSame(50101, $crc16->hash('key:002'));
+        $this->assertSame(54164, $crc16->hash('key:003'));
+        $this->assertSame(41843, $crc16->hash('key:004'));
+        $this->assertSame(45906, $crc16->hash('key:005'));
+        $this->assertSame(33585, $crc16->hash('key:006'));
+        $this->assertSame(37648, $crc16->hash('key:007'));
+        $this->assertSame(25343, $crc16->hash('key:008'));
+        $this->assertSame(29406, $crc16->hash('key:009'));
+    }
+}

tests/Predis/Connection/Aggregate/SentinelReplicationTest.php

@@ -44,8 +44,7 @@ class SentinelReplicationTest extends PredisTestCase
 
         $parameters = $replication->getSentinelConnection()->getParameters()->toArray();
 
-        $this->assertArrayNotHasKey('password', $parameters);
-        $this->assertArrayNotHasKey('database', $parameters);
+        $this->assertArraySubset(array('database' => null, 'password' => null), $parameters);
     }
 
     /**
@@ -552,6 +551,48 @@ class SentinelReplicationTest extends PredisTestCase
         $replication->connect();
     }
 
+    /**
+     * @group disconnected
+     */
+    public function testMethodConnectOnEmptySlavePoolAsksSentinelForSlavesAndForcesConnectionToMasterIfStillEmpty()
+    {
+        $sentinel1 = $this->getMockSentinelConnection('tcp://127.0.0.1:5381?alias=sentinel1');
+        $sentinel1->expects($this->at(0))
+                  ->method('executeCommand')
+                  ->with($this->isRedisCommand(
+                      'SENTINEL', array('slaves', 'svc')
+                  ))
+                  ->will($this->returnValue(
+                      array()
+                  ));
+        $sentinel1->expects($this->at(1))
+                  ->method('executeCommand')
+                  ->with($this->isRedisCommand(
+                      'SENTINEL', array('get-master-addr-by-name', 'svc')
+                  ))
+                  ->will($this->returnValue(
+                      array('127.0.0.1', '6381')
+                  ));
+
+        $master = $this->getMockConnection('tcp://127.0.0.1:6381?alias=master');
+        $master->expects($this->once())
+               ->method('connect');
+
+        $factory = $this->getMock('Predis\Connection\FactoryInterface');
+        $factory->expects($this->once())
+                 ->method('create')
+                 ->with(array(
+                    'host' => '127.0.0.1',
+                    'port' => '6381',
+                    'alias' => 'master',
+                  ))
+                 ->will($this->returnValue($master));
+
+        $replication = $this->getReplicationConnection('svc', array($sentinel1), $factory);
+
+        $replication->connect();
+    }
+
     /**
      * @group disconnected
      */