Permalink
Browse files

Eliminate Future from run-{all,app}.js, in favor of Promise.

Any code that has access to a Future object also has the capability of
calling its .return method. Promise objects, by contrast, can only be
resolved or rejected by the creator of the Promise (or any code granted
access to the special resolve and/or reject functions, which are not
simply methods of the Promise object). The run-app.js file contains a lot
of code that used to assume a Future could be resolved by anyone, which is
why the _{make,resolve}Promise methods were necessary.

For this reason, replacing Future with Promise in these two files seemed
tricky and worth attempting first, before spending time converting easier
files.
  • Loading branch information...
benjamn committed May 5, 2015
1 parent 16b3a3a commit fd42d6d6fa7ae27138688940d83016d210af31a0
Showing with 92 additions and 100 deletions.
  1. +17 −26 tools/run-all.js
  2. +75 −74 tools/run-app.js
View
@@ -1,6 +1,5 @@
var _ = require('underscore');
var Fiber = require('fibers');
var Future = require('fibers/future');
var files = require('./files.js');
var release = require('./release.js');
@@ -193,17 +192,13 @@ _.extend(Runner.prototype, {
_startMongoAsync: function () {
var self = this;
if (! self.stopped && self.mongoRunner) {
var future = new Future;
self.appRunner.awaitFutureBeforeStart(future);
var resolve = self.appRunner.makeBeforeStartPromise();
Fiber(function () {
self.mongoRunner.start();
if (! self.stopped && ! self.quiet) {
runLog.log("Started MongoDB.", { arrow: true });
}
// This future might also get resolved by appRunner.stop, so we need
// this check here (which is why we can't use f.future(), which does not
// have this check).
future.isResolved() || future.return();
resolve();
}).run();
}
},
@@ -294,43 +289,39 @@ exports.run = function (options) {
var once = runOptions.once;
delete runOptions.once;
var fut = new Future;
_.extend(runOptions, {
onFailure: function () {
var promise = new Promise(function (resolve) {
runOptions.onFailure = function () {
// Ensure that runner stops now. You might think this is unnecessary
// because the runner is stopped immediately after `fut.wait()`, but if
// because the runner is stopped immediately after promise.await(), but if
// the failure happens while runner.start() is still running, we want the
// rest of start to stop, and it's not like fut['return'] magically makes
// us jump to a fut.wait() that hasn't happened yet!.
// rest of start to stop, and it's not like resolve() magically makes
// us jump to a promise.await() that hasn't happened yet!.
runner.stop();
fut.isResolved() || fut['return']({ outcome: 'failure' });
},
onRunEnd: function (result) {
resolve({ outcome: 'failure' });
};
runOptions.onRunEnd = function (result) {
if (once ||
result.outcome === "conflicting-versions" ||
result.outcome === "wrong-release" ||
result.outcome === "outdated-cordova-platforms" ||
result.outcome === "outdated-cordova-plugins" ||
(result.outcome === "terminated" &&
result.signal === undefined && result.code === undefined)) {
// Allow run() to continue (and call runner.stop()) only once the
// AppRunner has processed our "return false"; otherwise we deadlock.
process.nextTick(function () {
fut.isResolved() || fut['return'](result);
});
resolve(result);
return false; // stop restarting
}
runner.regenerateAppPort();
return true; // restart it
},
watchForChanges: ! once,
quiet: once
};
});
runOptions.watchForChanges = ! once;
runOptions.quiet = once;
var runner = new Runner(runOptions);
runner.start();
var result = fut.wait();
var result = promise.await();
runner.stop();
if (result.outcome === "conflicting-versions") {
View
@@ -1,5 +1,4 @@
var _ = require('underscore');
var Future = require('fibers/future');
var Fiber = require('fibers');
var fiberHelpers = require('./fiber-helpers.js');
var files = require('./files.js');
@@ -372,14 +371,15 @@ var AppRunner = function (options) {
self.cordovaPlatforms = null;
self.fiber = null;
self.startFuture = null;
self.runFuture = null;
self.exitFuture = null;
self.watchFuture = null;
self.startPromise = null;
self.runPromise = null;
self.exitPromise = null;
self.watchPromise = null;
self._promiseResolvers = {};
// If this future is set with self.awaitFutureBeforeStart, then for the first
// If this promise is set with self.makeBeforeStartPromise, then for the first
// run, we will wait on it just before self.appProcess.start() is called.
self._beforeStartFuture = null;
self._beforeStartPromise = null;
// A hacky state variable that indicates that the proxy process (this process)
// is communicating to the app process over ipc. If an error in communication
// occurs, we can distinguish it in a callback handling the 'error' event.
@@ -395,15 +395,39 @@ _.extend(AppRunner.prototype, {
if (self.fiber)
throw new Error("already started?");
self.startFuture = new Future;
// XXX I think it's correct to not try to use bindEnvironment here:
// the extra fiber should be independent of this one.
self.startPromise = self._makePromise("start");
self.fiber = Fiber(function () {
self._fiber();
});
self.fiber.run();
self.startFuture.wait();
self.startFuture = null;
self.startPromise.await();
self.startPromise = null;
},
_makePromise: function (name) {
var self = this;
return new Promise(function (resolve) {
self._promiseResolvers[name] = resolve;
});
},
_resolvePromise: function (name, value) {
var resolve = this._promiseResolvers[name];
if (resolve) {
this._promiseResolvers[name] = null;
resolve(value);
}
},
_cleanUpPromises: function () {
if (this._promiseResolvers) {
_.each(this._promiseResolvers, function (resolve) {
resolve();
});
this._promiseResolvers = null;
}
},
// Shut down the app. stop() will block until the app is shut
@@ -416,32 +440,32 @@ _.extend(AppRunner.prototype, {
if (! self.fiber)
return; // nothing to do
if (self.exitFuture)
if (self.exitPromise)
throw new Error("another fiber already stopping?");
// The existence of this future makes the fiber break out of its loop.
self.exitFuture = new Future;
// The existence of this promise makes the fiber break out of its loop.
self.exitPromise = self._makePromise("exit");
self._runFutureReturn({ outcome: 'stopped' });
self._watchFutureReturn();
if (self._beforeStartFuture && ! self._beforeStartFuture.isResolved()) {
self._resolvePromise("run", { outcome: 'stopped' });
self._resolvePromise("watch");
if (self._beforeStartPromise) {
// If we stopped before mongod started (eg, due to mongod startup
// failure), unblock the runner fiber from waiting for mongod to start.
self._beforeStartFuture.return(true);
self._resolvePromise("beforeStart", true);
}
self.exitFuture.wait();
self.exitFuture = null;
self.exitPromise.await();
self.exitPromise = null;
},
awaitFutureBeforeStart: function(future) {
var self = this;
if (self._beforeStartFuture) {
throw new Error("awaitFutureBeforeStart called twice?");
} else if (future instanceof Future) {
self._beforeStartFuture = future;
} else {
throw new Error("non-Future passed to awaitFutureBeforeStart");
// Returns a function that can be called to resolve _beforeStartPromise.
makeBeforeStartPromise: function () {
if (this._beforeStartPromise) {
throw new Error("makeBeforeStartPromise called twice?");
}
this._beforeStartPromise = this._makePromise("beforeStart");
return this._promiseResolvers["beforeStart"];
},
// Run the program once, wait for it to exit, and then return. The
@@ -651,12 +675,12 @@ _.extend(AppRunner.prototype, {
}
// Atomically (1) see if we've been stop()'d, (2) if not, create a
// future that can be used to stop() us once we start running.
if (self.exitFuture)
// promise that can be used to stop() us once we start running.
if (self.exitPromise)
return { outcome: 'stopped' };
if (self.runFuture)
throw new Error("already have future?");
var runFuture = self.runFuture = new Future;
if (self.runPromise)
throw new Error("already have promise?");
var runPromise = self.runPromise = self._makePromise("run");
// Run the program
options.beforeRun && options.beforeRun();
@@ -670,7 +694,7 @@ _.extend(AppRunner.prototype, {
oplogUrl: self.oplogUrl,
mobileServerUrl: self.mobileServerUrl,
onExit: function (code, signal) {
self._runFutureReturn({
self._resolvePromise("run", {
outcome: 'terminated',
code: code,
signal: signal,
@@ -681,18 +705,16 @@ _.extend(AppRunner.prototype, {
onListen: function () {
self.proxy.setMode("proxy");
options.onListen && options.onListen();
if (self.startFuture)
self.startFuture['return']();
self._resolvePromise("start");
},
nodeOptions: getNodeOptionsFromEnvironment(),
nodePath: _.map(bundleResult.nodePath, files.convertToOSPath),
settings: settings,
ipcPipe: self.watchForChanges
});
// Empty self._beforeStartFutures and await its elements.
if (options.firstRun && self._beforeStartFuture) {
var stopped = self._beforeStartFuture.wait();
if (options.firstRun && self._beforeStartPromise) {
var stopped = self._beforeStartPromise.await();
if (stopped) {
return true;
}
@@ -711,7 +733,7 @@ _.extend(AppRunner.prototype, {
serverWatcher = new watch.Watcher({
watchSet: serverWatchSet,
onChange: function () {
self._runFutureReturn({
self._resolvePromise("run", {
outcome: 'changed'
});
}
@@ -726,7 +748,7 @@ _.extend(AppRunner.prototype, {
var outcome = watch.isUpToDate(serverWatchSet)
? 'changed-refreshable' // only a client asset has changed
: 'changed'; // both a client and server asset changed
self._runFutureReturn({ outcome: outcome });
self._resolvePromise('run', { outcome: outcome });
}
});
};
@@ -738,7 +760,7 @@ _.extend(AppRunner.prototype, {
// Wait for either the process to exit, or (if watchForChanges) a
// source file to change. Or, for stop() to be called.
var ret = runFuture.wait();
var ret = runPromise.await();
try {
while (ret.outcome === 'changed-refreshable') {
@@ -752,7 +774,7 @@ _.extend(AppRunner.prototype, {
return bundleResultOrRunResult.runResult;
bundleResult = bundleResultOrRunResult.bundleResult;
var oldFuture = self.runFuture = new Future;
var oldPromise = self.runPromise = self._makePromise("run");
// Notify the server that new client assets have been added to the
// build.
@@ -766,10 +788,10 @@ _.extend(AppRunner.prototype, {
runLog.logClientRestart();
// Wait until another file changes.
ret = oldFuture.wait();
ret = oldPromise.await();
}
} finally {
self.runFuture = null;
self.runPromise = null;
if (ret.outcome === 'changed') {
runLog.logTemporary("=> Server modified -- restarting...");
@@ -785,24 +807,6 @@ _.extend(AppRunner.prototype, {
return ret;
},
_runFutureReturn: function (value) {
var self = this;
if (!self.runFuture)
return;
var runFuture = self.runFuture;
self.runFuture = null;
runFuture['return'](value);
},
_watchFutureReturn: function () {
var self = this;
if (!self.watchFuture)
return;
var watchFuture = self.watchFuture;
self.watchFuture = null;
watchFuture.return();
},
_fiber: function () {
var self = this;
@@ -833,7 +837,7 @@ _.extend(AppRunner.prototype, {
crashCount = 0;
var wantExit = self.onRunEnd ? !self.onRunEnd(runResult) : false;
if (wantExit || self.exitFuture || runResult.outcome === "stopped")
if (wantExit || self.exitPromise || runResult.outcome === "stopped")
break;
if (runResult.outcome === "wrong-release" ||
@@ -885,22 +889,22 @@ _.extend(AppRunner.prototype, {
}
if (self.watchForChanges) {
self.watchFuture = new Future;
self.watchPromise = self._makePromise("watch");
if (!runResult.watchSet)
throw Error("watching for changes with no watchSet?");
var watcher = new watch.Watcher({
watchSet: runResult.watchSet,
onChange: function () {
self._watchFutureReturn();
self._resolvePromise("watch");
}
});
self.proxy.setMode("errorpage");
// If onChange wasn't called synchronously (clearing watchFuture), wait
// If onChange wasn't called synchronously (clearing watchPromise), wait
// on it.
self.watchFuture && self.watchFuture.wait();
self.watchPromise && self.watchPromise.await();
// While we were waiting, did somebody stop() us?
if (self.exitFuture)
if (self.exitPromise)
break;
runLog.log("Modified -- restarting.", { arrow: true });
Console.enableProgressDisplay(true);
@@ -911,10 +915,7 @@ _.extend(AppRunner.prototype, {
}
// Giving up for good.
if (self.exitFuture)
self.exitFuture['return']();
if (self.startFuture)
self.startFuture['return']();
self._cleanUpPromises();
self.fiber = null;
}

0 comments on commit fd42d6d

Please sign in to comment.