From 2b8f2cc566cfaf3369f17b1da9ec60f601587b91 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@meteor.com>
Date: Tue, 29 Apr 2014 14:20:51 -0700
Subject: [PATCH] Handle token observe better with overlapping login

Before this, we could see the "non-null user observe" error if:

- One login method ran (eg login) and it called _setLoginToken.
  It stored null in userObservesForConnections and gets to the
  defer/observe part
- Another login method ran (eg getNewToken) and it called
  _setLoginToken. The call to removeTokenFromConnection at the top
  clears the null from userObservesForConnections, and it then
  stores its own null in userObservesForConnections, and defers
- One of them finishes the observe and puts its observe in
  userObservesForConnections, overwriting the null which it thinks
  is its alone
- The other one gets there and throws

Also, consistently use _.has when checking if userObservesForConnections
has an element.
---
 packages/accounts-base/accounts_server.js | 30 ++++++++++++++---------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js
index 5f4de80932..4bb346a578 100644
--- a/packages/accounts-base/accounts_server.js
+++ b/packages/accounts-base/accounts_server.js
@@ -644,8 +644,8 @@ Accounts._getUserObserve = function (connectionId) {
 // the observe that we started when we associated the connection with
 // this token.
 var removeTokenFromConnection = function (connectionId) {
-  var observe = userObservesForConnections[connectionId];
-  if (observe !== undefined) {
+  if (_.has(userObservesForConnections, connectionId)) {
+    var observe = userObservesForConnections[connectionId];
     if (observe === null) {
       // We're in the process of setting up an observe for this
       // connection. We can't clean up that observe yet, but if we
@@ -703,18 +703,26 @@ Accounts._setLoginToken = function (userId, connection, newToken) {
         }
       });
 
-      if (_.has(userObservesForConnections, connection.id)) {
-        if (userObservesForConnections[connection.id] !== null) {
-          throw new Error("Non-null user observe for connection " +
-                          connection.id + " while observe was being set up?");
-        }
-        userObservesForConnections[connection.id] = observe;
-      } else {
-        // Oops, this connection was closed while we were setting up the
-        // observe. Clean it up now.
+      // If the user ran another login or logout command we were waiting for
+      // the defer or added to fire, then we let the later one win (start an
+      // observe, etc) and just stop our observe now.
+      //
+      // Similarly, if the connection was already closed, then the onClose
+      // callback would have called removeTokenFromConnection and there won't be
+      // an entry in userObservesForConnections. We can stop the observe.
+      if (Accounts._getAccountData(connection.id, 'loginToken') !== newToken ||
+          !_.has(userObservesForConnections, connection.id)) {
         observe.stop();
+        return;
       }
 
+      if (userObservesForConnections[connection.id] !== null) {
+        throw new Error("Non-null user observe for connection " +
+                        connection.id + " while observe was being set up?");
+      }
+
+      userObservesForConnections[connection.id] = observe;
+
       if (! foundMatchingUser) {
         // We've set up an observe on the user associated with `newToken`,
         // so if the new token is removed from the database, we'll close