Permalink
Browse files

Fix bogus handling of error replies.

Error replies returned by Redis were erroneusly triggering the automatic
reconnection of the client thus leading to infinite loops.

An automatic reconnection mechanism probably sounds nice at first but can
easily lead to unexpected behaviours, and considered that we don't support
them in the binary-safe variants of the commands function it's better to
drop this feature altogether.

This commit fixes issue #12.
  • Loading branch information...
1 parent 46b4025 commit 405b7631a8bf97a7e54556378ce1fe4cd9fea282 @nrk committed Oct 22, 2013
Showing with 16 additions and 24 deletions.
  1. +14 −22 phpiredis.c
  2. +2 −2 tests/008.phpt
View
@@ -722,30 +722,22 @@ PHP_FUNCTION(phpiredis_command)
ZEND_FETCH_RESOURCE2(connection, phpiredis_connection *, &resource, -1, PHPIREDIS_CONNECTION_NAME, le_redis_context, le_redis_persistent_context);
- while (reply == NULL || reply->type == REDIS_REPLY_ERROR)
- {
- reply = redisCommand(connection->c,command);
- if (reply == NULL) {
- redisFree(connection->c);
- redisContext* c = redisConnect(connection->ip, connection->port);
+ reply = redisCommand(connection->c, command);
- if (c->err) {
- redisFree(c);
- connection->c = NULL;
- RETURN_FALSE;
- return;
- }
- connection->c = c;
- } else if (reply->type == REDIS_REPLY_ERROR) {
- if (connection->c == REDIS_OK) { // The problem was the command
- php_error_docref(NULL TSRMLS_CC, E_WARNING, reply->str);
- RETURN_FALSE;
- return;
- } else {
- // TODO: whats happening here?
- }
- }
+ if (reply == NULL) {
+ RETURN_FALSE;
+ return;
}
+
+ if (reply->type == REDIS_REPLY_ERROR) {
+ php_error_docref(NULL TSRMLS_CC, E_WARNING, reply->str);
+
+ freeReplyObject(reply);
+
+ RETURN_FALSE;
+ return;
+ }
+
convert_redis_to_php(NULL, return_value, reply);
freeReplyObject(reply);
}
View
@@ -1,5 +1,5 @@
--TEST--
-phpiredis reconnect on disconnect
+phpiredis does not reconnect on disconnect
--SKIPIF--
<?php include 'skipif.inc'; ?>
--FILE--
@@ -20,4 +20,4 @@ var_dump(phpiredis_command($link, 'GET a'));
--EXPECTF--
string(1) "1"
string(2) "OK"
-string(1) "1"
+bool(false)

0 comments on commit 405b763

Please sign in to comment.