fix: race conditions in Redis#disconnect() can cancel reconnection …

…unexpectedly Closes #1138, and closes #1007
redis · May 30, 2020 · 6fad73b · 6fad73b
1 parent 9721ebd
commit 6fad73b
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 6 deletions.
diff --git a/lib/cluster/index.ts b/lib/cluster/index.ts
@@ -316,7 +316,7 @@ class Cluster extends EventEmitter {
     if (!reconnect) {
       this.manuallyClosing = true;
     }
-    if (this.reconnectTimeout) {
+    if (this.reconnectTimeout && !reconnect) {
       clearTimeout(this.reconnectTimeout);
       this.reconnectTimeout = null;
       debug("Canceled reconnecting attempts");

diff --git a/lib/redis/index.ts b/lib/redis/index.ts
@@ -278,10 +278,9 @@ Redis.prototype.connect = function (callback) {
 
     this.condition = {
       select: options.db,
-      auth:
-        options.username
-          ? [options.username, options.password]
-          : options.password,
+      auth: options.username
+        ? [options.username, options.password]
+        : options.password,
       subscriber: false,
     };
 
@@ -380,7 +379,7 @@ Redis.prototype.disconnect = function (reconnect) {
   if (!reconnect) {
     this.manuallyClosing = true;
   }
-  if (this.reconnectTimeout) {
+  if (this.reconnectTimeout && !reconnect) {
     clearTimeout(this.reconnectTimeout);
     this.reconnectTimeout = null;
   }

diff --git a/test/functional/cluster/connect.ts b/test/functional/cluster/connect.ts
@@ -384,4 +384,30 @@ describe("cluster:connect", function () {
       }
     });
   });
+
+  describe("multiple reconnect", function () {
+    it("should reconnect after multiple consecutive disconnect(true) are called", function (done) {
+      new MockServer(30001);
+      const cluster = new Cluster([{ host: "127.0.0.1", port: "30001" }], {
+        enableReadyCheck: false,
+      });
+      cluster.once("reconnecting", function () {
+        cluster.disconnect(true);
+      });
+      cluster.once("ready", function () {
+        cluster.disconnect(true);
+        const rejectTimeout = setTimeout(function () {
+          cluster.disconnect();
+          done(new Error("second disconnect(true) didn't reconnect redis"));
+        }, 1000);
+        process.nextTick(function () {
+          cluster.once("ready", function () {
+            clearTimeout(rejectTimeout);
+            cluster.disconnect();
+            done();
+          });
+        });
+      });
+    });
+  });
 });
diff --git a/test/functional/connection.ts b/test/functional/connection.ts
@@ -451,4 +451,27 @@ describe("connection", function () {
       });
     });
   });
+
+  describe("multiple reconnect", function () {
+    it("should reconnect after multiple consecutive disconnect(true) are called", function (done) {
+      const redis = new Redis();
+      redis.once("reconnecting", function () {
+        redis.disconnect(true);
+      });
+      redis.once("ready", function () {
+        redis.disconnect(true);
+        const rejectTimeout = setTimeout(function () {
+          redis.disconnect();
+          done(new Error("second disconnect(true) didn't reconnect redis"));
+        }, 1000);
+        process.nextTick(function () {
+          redis.once("ready", function () {
+            clearTimeout(rejectTimeout);
+            redis.disconnect();
+            done();
+          });
+        });
+      });
+    });
+  });
 });