🐛 Fix Presence.destroy() race condition

There's currently a race condition that can throw uncaught errors when
destroying both a `LocalPresence` *and* its parent `Presence` object.

If the `LocalPresence` destroys first, then - since the `Presence`
[cached][1] its ID before the `Presence` unsubscribe - by the time the
`Presence` tries to destroy its children, the `LocalPresence` is already
gone, which throws when [trying to call `destroy()`][2].

This change adds a test for this race condition, and adds a fix, which
just checks if the `LocalPresence` still exists on the parent before
trying to destroy it.

[1]: https://github.com/share/sharedb/blob/91cae28a0065faf68749243ef36aadf54bf0645d/lib/client/presence/presence.js#L56
[2]: https://github.com/share/sharedb/blob/91cae28a0065faf68749243ef36aadf54bf0645d/lib/client/presence/presence.js#L64

Author: alecgibson

lib/client/presence/presence.js

@@ -61,7 +61,9 @@ Presence.prototype.destroy = function(callback) {
       [
         function(next) {
           async.each(localIds, function(presenceId, next) {
-            presence.localPresences[presenceId].destroy(next);
+            var localPresence = presence.localPresences[presenceId];
+            if (!localPresence) return next();
+            localPresence.destroy(next);
           }, next);
         },
         function(next) {

test/client/presence/presence.js

@@ -146,6 +146,32 @@ describe('Presence', function() {
     ], errorHandler(done));
   });
 
+  it('does not throw if LocalPresence destroys before Presence', function(done) {
+    var localPresence1 = presence1.create('presence-1');
+
+    async.series([
+      presence1.subscribe.bind(presence1),
+      function(next) {
+        // Middleware that ensures the presence update replies before the
+        // unsubscribe, so the local presence is destroyed first
+        var replies = [];
+        backend.use('reply', function(message, cb) {
+          if (!replies) return cb();
+          if (message.reply.a !== 'p') return replies.push(cb);
+          var _replies = replies;
+          replies = null;
+          cb();
+          _replies.forEach(function(reply) {
+            reply();
+          });
+        });
+
+        presence1.destroy(next);
+        localPresence1.destroy(errorHandler(done));
+      }
+    ], done);
+  });
+
   it('throws if trying to create local presence when wanting destroy', function(done) {
     presence2.destroy(errorHandler(done));
     expect(function() {