Short term additional fix for #583

The previous commits do not handle all of the edge cases of #583
appropriately.  This is a short term solution that will work, but is not
as efficient as it could be.  The whole refreshAccount function needs to
be reconsidered and replaced with a more sane way of handling atomic
updates to the user's account state.

package.json

@@ -2,7 +2,7 @@
   "author": "Calvin Montgomery",
   "name": "CyTube",
   "description": "Online media synchronizer and chat",
-  "version": "3.17.3",
+  "version": "3.17.4",
   "repository": {
     "url": "http://github.com/calzoneman/sync"
   },

src/channel/channel.js

@@ -322,11 +322,9 @@ Channel.prototype.joinUser = function (user, data) {
             return;
         }
 
-        function refreshUserAccount(cb) {
+        user.channel = self;
-            user.refreshAccount({
+        if (self.is(Flags.C_REGISTERED)) {
-                channel: self.name,
+            user.refreshAccount(function (err, account) {
-                name: user.getName()
-            }, function (err, account) {
                 if (err) {
                     Logger.errlog.log("user.refreshAccount failed at Channel.joinUser");
                     Logger.errlog.log(err.stack);
@@ -334,19 +332,8 @@ Channel.prototype.joinUser = function (user, data) {
                     return;
                 }
 
-                if (cb) {
+                afterAccount();
-                    process.nextTick(cb);
-                }
             });
-        }
-
-        if (self.is(Flags.C_REGISTERED) && user.is(Flags.U_LOGGED_IN)) {
-            refreshUserAccount(afterAccount);
-        } else if (self.is(Flags.C_REGISTERED)) {
-            user.waitFlag(Flags.U_LOGGED_IN, () => {
-                refreshUserAccount();
-            });
-            afterAccount();
         } else {
             afterAccount();
         }
@@ -361,11 +348,9 @@ Channel.prototype.joinUser = function (user, data) {
 
             self.checkModules("onUserPreJoin", [user, data], function (err, result) {
                 if (result === ChannelModule.PASSTHROUGH) {
-                    if (user.account.channelRank !== user.account.globalRank) {
-                        user.socket.emit("rank", user.account.effectiveRank);
-                    }
                     self.acceptUser(user);
                 } else {
+                    user.channel = null;
                     user.account.channelRank = 0;
                     user.account.effectiveRank = user.account.globalRank;
                     self.refCounter.unref("Channel::user");
@@ -376,7 +361,6 @@ Channel.prototype.joinUser = function (user, data) {
 };
 
 Channel.prototype.acceptUser = function (user) {
-    user.channel = this;
     user.setFlag(Flags.U_IN_CHANNEL);
     user.socket.join(this.name);
     user.autoAFK();

src/io/ioserver.js

@@ -213,8 +213,8 @@ function handleConnection(sock) {
     if (sock.user) {
         user.setFlag(Flags.U_REGISTERED);
         user.clearFlag(Flags.U_READY);
-        user.refreshAccount({ name: sock.user.name },
+        user.account.name = sock.user.name;
-                            function (err, account) {
+        user.refreshAccount(function (err, account) {
             if (err) {
                 user.clearFlag(Flags.U_REGISTERED);
                 user.setFlag(Flags.U_READY);

src/user.js

@@ -177,6 +177,10 @@ User.prototype.inChannel = function () {
     return this.channel != null && !this.channel.dead;
 };
 
+User.prototype.inRegisteredChannel = function () {
+    return this.inChannel() && this.channel.is(Flags.C_REGISTERED);
+};
+
 /* Called when a user's AFK status changes */
 User.prototype.setAFK = function (afk) {
     if (!this.inChannel()) {
@@ -283,17 +287,15 @@ User.prototype.login = function (name, pw) {
             return;
         }
 
-        var opts = { name: user.name };
+        self.account.name = user.name;
-        if (self.inChannel()) {
-            opts.channel = self.channel.name;
-        }
         self.setFlag(Flags.U_REGISTERED);
-        self.refreshAccount(opts, function (err, account) {
+        self.refreshAccount(function (err, account) {
             if (err) {
                 Logger.errlog.log("[SEVERE] getAccount failed for user " + user.name);
                 Logger.errlog.log(err);
                 self.clearFlag(Flags.U_REGISTERED);
                 self.clearFlag(Flags.U_LOGGING_IN);
+                self.account.name = "";
                 return;
             }
             self.socket.emit("login", {
@@ -370,14 +372,12 @@ User.prototype.guestLogin = function (name) {
         // Login succeeded
         lastguestlogin[self.realip] = Date.now();
 
-        var opts = { name: name };
+        self.account.name = name;
-        if (self.inChannel()) {
+        self.refreshAccount(function (err, account) {
-            opts.channel = self.channel.name;
-        }
-        self.refreshAccount(opts, function (err, account) {
             if (err) {
                 Logger.errlog.log("[SEVERE] getAccount failed for guest login " + name);
                 Logger.errlog.log(err);
+                self.account.name = "";
                 return;
             }
 
@@ -409,29 +409,28 @@ setInterval(function () {
     }
 }, 5 * 60 * 1000);
 
-User.prototype.refreshAccount = function (opts, cb) {
+User.prototype.refreshAccount = function (cb) {
-    if (!cb) {
+    var name = this.account.name;
-        cb = opts;
+    var opts = {
-        opts = {};
+        registered: this.is(Flags.U_REGISTERED),
-    }
+        channel: this.inRegisteredChannel() ? this.channel.name : false
-
+    };
-    var different = false;
-    for (var key in opts) {
-        if (opts[key] !== this.account[key]) {
-            different = true;
-            break;
-        }
-    }
-
-    if (!different) {
-        return;
-    }
-
-    var name = ("name" in opts) ? opts.name : this.account.name;
-    opts.registered = this.is(Flags.U_REGISTERED);
     var self = this;
     var old = this.account;
     Account.getAccount(name, this.realip, opts, function (err, account) {
+        // TODO
+        //
+        // This is a hack to fix #583, an issue where racing callbacks
+        // from refreshAccount() can cause the user's rank to get out
+        // of sync.  Ideally this should be removed in favor of a more
+        // robust way of handling updating account state, perhaps a mutex.
+        if (self.is(Flags.U_REGISTERED) !== opts.registered ||
+                (self.inRegisteredChannel() && !opts.channel) ||
+                self.account.name !== name) {
+            self.refreshAccount(cb);
+            return;
+        }
+
         if (!err) {
             /* Update account if anything changed in the meantime */
             for (var key in old) {
@@ -445,7 +444,8 @@ User.prototype.refreshAccount = function (opts, cb) {
                 self.emit("effectiveRankChange", self.account.effectiveRank);
             }
         }
-        cb(err, account);
+
+        process.nextTick(cb, err, account);
     });
 };