From ae68f4bee81e8182bd9520e2b7b5f796fa2ba3c8 Mon Sep 17 00:00:00 2001 From: Simon Woolf Date: Wed, 20 Jul 2016 22:00:01 +0100 Subject: [PATCH 1/2] ConnectionManager: fix a few race conditions eg retryImmediately assumed that the state wouldn't change from disconnected in the tick before it tries to connect again, which was problematic if the connection was closed in that tick. --- common/lib/transport/connectionmanager.js | 25 +++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/common/lib/transport/connectionmanager.js b/common/lib/transport/connectionmanager.js index 872fa23859..7fd5447c13 100644 --- a/common/lib/transport/connectionmanager.js +++ b/common/lib/transport/connectionmanager.js @@ -460,8 +460,11 @@ var ConnectionManager = (function() { * connection event, then we won't activate this transport */ var existingState = this.state; Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.activateTransport()', 'current state = ' + existingState.state); - if(existingState.state == this.states.closing.state || existingState.state == this.states.closed.state || existingState.state == this.states.failed.state) + if(existingState.state == this.states.closing.state || existingState.state == this.states.closed.state || existingState.state == this.states.failed.state) { + Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.activateTransport()', 'Disconnecting transport and abandoning'); + transport.disconnect(); return false; + } /* remove this transport from pending transports */ Utils.arrDeleteValue(this.pendingTransports, transport); @@ -575,8 +578,11 @@ var ConnectionManager = (function() { } else if(wasActive) { /* If we were active but there is another transport scheduled for * activation, go into to the connecting state until that transport - * activates and sets us back to connected */ + * activates and sets us back to connected. (manually starting the + * transition timers in case that never happens) */ Logger.logAction(Logger.LOG_MICRO, 'ConnectionManager.deactivateTransport()', 'wasActive but another transport is connected and scheduled for activation, so going into the connecting state until it activates'); + this.startSuspendTimer(); + this.startTransitionTimer(this.states.connecting); this.notifyState({state: 'connecting', error: error}); } }; @@ -709,7 +715,7 @@ var ConnectionManager = (function() { this.transitionTimer = setTimeout(function() { if(self.transitionTimer) { self.transitionTimer = null; - Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager connect timer expired', 'requesting new state: ' + self.states.connecting.failState); + Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager ' + transitionState.state + ' timer expired', 'requesting new state: ' + transitionState.failState); self.notifyState({state: transitionState.failState}); } }, transitionState.retryDelay); @@ -782,7 +788,7 @@ var ConnectionManager = (function() { (this.state === this.states.connecting && indicated.error && Auth.isTokenErr(indicated.error)))); - Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.notifyState()', 'new state: ' + state); + Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.notifyState()', 'new state: ' + state + (retryImmediately ? '; will retry connection immediately' : '')); /* do nothing if we're already in the indicated state */ if(state == this.state.state) return; @@ -803,9 +809,11 @@ var ConnectionManager = (function() { if(retryImmediately) { Utils.nextTick(function() { - self.requestState({state: 'connecting'}); + if(self.state === self.states.disconnected) { + self.requestState({state: 'connecting'}); + } }); - } else if(newState.retryDelay) { + } else if(state === 'disconnected' || state === 'suspended') { this.startRetryTimer(newState.retryDelay); } @@ -864,6 +872,11 @@ var ConnectionManager = (function() { ConnectionManager.prototype.startConnect = function() { + if(this.state !== this.states.connecting) { + Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.startConnect()', 'Must be in connecting state to connect, but was ' + this.state.state); + return; + } + var auth = this.realtime.auth, self = this; From 21e859a1b2a27cdcf7378798ae4d4c0968c1a99c Mon Sep 17 00:00:00 2001 From: Simon Woolf Date: Mon, 25 Jul 2016 12:10:39 +0100 Subject: [PATCH 2/2] Rate-limit autoreconnects to max 1 per second --- common/lib/transport/connectionmanager.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/common/lib/transport/connectionmanager.js b/common/lib/transport/connectionmanager.js index 7fd5447c13..98637d6ece 100644 --- a/common/lib/transport/connectionmanager.js +++ b/common/lib/transport/connectionmanager.js @@ -127,6 +127,7 @@ var ConnectionManager = (function() { this.proposedTransports = []; this.pendingTransports = []; this.host = null; + this.lastAutoReconnectAttempt = null; Logger.logAction(Logger.LOG_MINOR, 'Realtime.ConnectionManager()', 'started'); Logger.logAction(Logger.LOG_MICRO, 'Realtime.ConnectionManager()', 'requested transports = [' + (options.transports || Defaults.transports) + ']'); @@ -808,11 +809,19 @@ var ConnectionManager = (function() { change = new ConnectionStateChange(this.state.state, newState.state, newState.retryDelay, (indicated.error || ConnectionError[newState.state])); if(retryImmediately) { - Utils.nextTick(function() { + var autoReconnect = function() { if(self.state === self.states.disconnected) { + self.lastAutoReconnectAttempt = Utils.now(); self.requestState({state: 'connecting'}); } - }); + }; + var sinceLast = this.lastAutoReconnectAttempt && (Utils.now() - this.lastAutoReconnectAttempt + 1); + if(sinceLast && (sinceLast < 1000)) { + Logger.logAction(Logger.LOG_MICRO, 'ConnectionManager.notifyState()', 'Last reconnect attempt was only ' + sinceLast + 'ms ago, waiting another ' + (1000 - sinceLast) + 'ms before trying again'); + setTimeout(autoReconnect, 1000 - sinceLast); + } else { + Utils.nextTick(autoReconnect); + } } else if(state === 'disconnected' || state === 'suspended') { this.startRetryTimer(newState.retryDelay); }