From 2ae1ea495f8cc649afa2454b2f5fbd41a42b5fcc Mon Sep 17 00:00:00 2001 From: Andrew Wilcox Date: Fri, 15 Nov 2013 09:52:18 -0500 Subject: [PATCH] Wrap calling session close callback in Meteor.defer, so that a bunch of connections can be closed without waiting for the close callbacks on one connection to return before closing the other connections. Underscore internal Session field `_closeCallbacks`. Update comment to explain the cause of the problem requiring the use of `Meteor.bindEnvironment` with Meteor's public API. --- packages/livedata/livedata_server.js | 11 ++++++----- packages/livedata/livedata_server_tests.js | 10 ++++++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js index f9e93a5f23..0f8b9ee47d 100644 --- a/packages/livedata/livedata_server.js +++ b/packages/livedata/livedata_server.js @@ -252,7 +252,7 @@ var Session = function (server, version, socket) { self._pendingReady = []; // List of callbacks to call when this session is closed. - self.closeCallbacks = []; + self._closeCallbacks = []; // The `SessionHandle` for this session, passed to // `Meteor.server.onConnection` callbacks. @@ -271,7 +271,7 @@ var Session = function (server, version, socket) { ); } ); - self.closeCallbacks.push(fn); + self._closeCallbacks.push(fn); }, _sessionData: self.sessionData }; @@ -397,9 +397,10 @@ _.extend(Session.prototype, { // Drop the merge box data immediately. self.collectionViews = {}; self.inQueue = null; - // XXX do we need to use Meteor.defer here as well? - _.each(self.closeCallbacks, function (callback) { - callback(); + Meteor.defer(function () { + _.each(self._closeCallbacks, function (callback) { + callback(); + }); }); Package.facts && Package.facts.Facts.incrementServerFact( "livedata", "sessions", -1); diff --git a/packages/livedata/livedata_server_tests.js b/packages/livedata/livedata_server_tests.js index 2d00e4f4d6..dd868521ef 100644 --- a/packages/livedata/livedata_server_tests.js +++ b/packages/livedata/livedata_server_tests.js @@ -20,10 +20,12 @@ Tinytest.addAsync( "livedata server - sessionHandle.close()", function (test, onComplete) { - // XXX I don't understand why using `bindEnvironment` here is - // necessary, but I get "Meteor code must always run within a - // Fiber. Try wrapping callbacks that you pass to non-Meteor - // libraries with Meteor.bindEnvironment" if I don't. + // XXX stream_client_nodejs.js should not be requiring a developer + // to use Meteor.bindEnvironment themselves when using Meteor's + // public API. The problem is that the computation rerunning is + // triggered by the close event firing on the stream's connection + // object, and that callback in stream_client_nodejs.js is not + // wrapped in a Meteor.bindEnvironment for us. done = Meteor.bindEnvironment( function () { Meteor.defer(onComplete);