From 6fad73b672014c07bd0db7a8e51c0be341908868 Mon Sep 17 00:00:00 2001 From: Alexandre ABRIOUX Date: Sat, 30 May 2020 06:05:37 +0200 Subject: [PATCH] fix: race conditions in `Redis#disconnect()` can cancel reconnection unexpectedly Closes #1138, and closes #1007 --- lib/cluster/index.ts | 2 +- lib/redis/index.ts | 9 ++++----- test/functional/cluster/connect.ts | 26 ++++++++++++++++++++++++++ test/functional/connection.ts | 23 +++++++++++++++++++++++ 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/lib/cluster/index.ts b/lib/cluster/index.ts index 0129ee46..8b8d1f20 100644 --- 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 index fae6eb9a..0ff916a8 100644 --- 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 index f3bc9fc6..bb794aad 100644 --- 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 index 78f48893..34e2aac7 100644 --- 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(); + }); + }); + }); + }); + }); });