Merge pull request phpredis#1108 from yatsukhnenko/develop

yatsukhnenko · web-flow · commit ff549abcf638 · 2017-02-17T22:26:58.000+02:00
redis_check_eof refactoring
diff --git a/library.c b/library.c
@@ -140,10 +140,10 @@ PHP_REDIS_API void redis_stream_close(RedisSock *redis_sock TSRMLS_DC) {
     }
 }
 
-PHP_REDIS_API int redis_check_eof(RedisSock *redis_sock, int no_throw TSRMLS_DC)
+PHP_REDIS_API int
+redis_check_eof(RedisSock *redis_sock, int no_throw TSRMLS_DC)
 {
-    int eof;
-    int count = 0;
+    int count;
 
     if (!redis_sock->stream) {
         return -1;
@@ -164,53 +164,57 @@ PHP_REDIS_API int redis_check_eof(RedisSock *redis_sock, int no_throw TSRMLS_DC)
      * Bug fix of php: https://github.com/php/php-src/pull/1456
      * */
     errno = 0;
-    eof = php_stream_eof(redis_sock->stream);
-    for (; eof; count++) {
-        if((MULTI == redis_sock->mode) || redis_sock->watching || count == 10) {
-            /* too many failures */
-            if(redis_sock->stream) { /* close stream if still here */
-                REDIS_STREAM_CLOSE_MARK_FAILED(redis_sock);
-            }
-            if(!no_throw) {
-                zend_throw_exception(redis_exception_ce, "Connection lost", 
-                    0 TSRMLS_CC);
-            }
-        return -1;
+    if (php_stream_eof(redis_sock->stream) == 0) {
+        /* Success */
+        return 0;
+    } else if (redis_sock->mode == MULTI || redis_sock->watching) {
+        REDIS_STREAM_CLOSE_MARK_FAILED(redis_sock);
+        if (!no_throw) {
+            zend_throw_exception(redis_exception_ce,
+                "Connection lost and socket is in MULTI/watching mode",
+                0 TSRMLS_CC);
         }
-        if(redis_sock->stream) { /* close existing stream before reconnecting */
+        return -1;
+    }
+    /* TODO: configurable max retry count */
+    for (count = 0; count < 10; ++count) {
+        /* close existing stream before reconnecting */
+        if (redis_sock->stream) {
             redis_stream_close(redis_sock TSRMLS_CC);
             redis_sock->stream = NULL;
-            redis_sock->mode   = ATOMIC;
-            redis_sock->watching = 0;
         }
         // Wait for a while before trying to reconnect
         if (redis_sock->retry_interval) {
             // Random factor to avoid having several (or many) concurrent connections trying to reconnect at the same time
             long retry_interval = (count ? redis_sock->retry_interval : (php_rand(TSRMLS_C) % redis_sock->retry_interval));
             usleep(retry_interval);
         }
-        redis_sock_connect(redis_sock TSRMLS_CC); /* reconnect */
-        if(redis_sock->stream) { /*  check for EOF again. */
+        /* reconnect */
+        if (redis_sock_connect(redis_sock TSRMLS_CC) == 0) {
+            /* check for EOF again. */
             errno = 0;
-            eof = php_stream_eof(redis_sock->stream);
+            if (php_stream_eof(redis_sock->stream) == 0) {
+                /* If we're using a password, attempt a reauthorization */
+                if (redis_sock->auth && resend_auth(redis_sock TSRMLS_CC) != 0) {
+                    break;
+                }
+                /* If we're using a non-zero db, reselect it */
+                if (redis_sock->dbNumber && reselect_db(redis_sock TSRMLS_CC) != 0) {
+                    break;
+                }
+                /* Success */
+                return 0;
+            }
         }
     }
-
-    /* We've connected if we have a count */
-    if (count) {
-        /* If we're using a password, attempt a reauthorization */
-        if (redis_sock->auth && resend_auth(redis_sock TSRMLS_CC) != 0) {
-            return -1;
-        }
-
-        /* If we're using a non-zero db, reselect it */
-        if (redis_sock->dbNumber && reselect_db(redis_sock TSRMLS_CC) != 0) {
-            return -1;
-        }
+    /* close stream if still here */
+    if (redis_sock->stream) {
+        REDIS_STREAM_CLOSE_MARK_FAILED(redis_sock);
     }
-
-    /* Success */
-    return 0;
+    if (!no_throw) {
+        zend_throw_exception(redis_exception_ce, "Connection lost", 0 TSRMLS_CC);
+    }
+    return -1;
 }
 
 
diff --git a/redis.c b/redis.c
@@ -844,6 +844,7 @@ redis_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
 
     if (redis_sock_server_open(redis->sock, 1 TSRMLS_CC) < 0) {
         redis_free_socket(redis->sock);
+        redis->sock = NULL;
         return FAILURE;
     }
 

Original file line number	Diff line number	Diff line change
`@@ -844,6 +844,7 @@ redis_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)`
`844`	`844`
`845`	`845`	`if (redis_sock_server_open(redis->sock, 1 TSRMLS_CC) < 0) {`
`846`	`846`	`redis_free_socket(redis->sock);`
	`847`	`+ redis->sock = NULL;`
`847`	`848`	`return FAILURE;`
`848`	`849`	`}`
`849`	`850`