From efe34f502374d273cd28601b338df734059657ab Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 16 Oct 2009 14:55:30 +0200 Subject: [PATCH] Only allow a promise to fire once, remove promise.cancel() promise.cancel() is due to return at some point. --- doc/api.txt | 16 +--------------- src/events.js | 24 ------------------------ src/node_events.cc | 6 ++++++ src/node_events.h | 2 ++ test/mjsunit/test-promise-cancel.js | 27 --------------------------- 5 files changed, 9 insertions(+), 66 deletions(-) delete mode 100644 test/mjsunit/test-promise-cancel.js diff --git a/doc/api.txt b/doc/api.txt index 0f0cfd64d..1c49946fc 100644 --- a/doc/api.txt +++ b/doc/api.txt @@ -215,7 +215,6 @@ emit anymore events. | Event | Parameters | Notes | +"success"+ | (depends) | | +"error"+ | (depends) | -| +"cancel"+ | (depends) | |========================================================= +promise.addCallback(listener)+ :: @@ -224,9 +223,6 @@ Adds a listener for the +"success"+ event. Returns the same promise object. +promise.addErrback(listener)+ :: Adds a listener for the +"error"+ event. Returns the same promise object. -+promise.addCancelback(listener)+ :: -Adds a listener for the +"cancel"+ event. Returns the same promise object. - +promise.emitSuccess(arg1, arg2, ...)+ :: If you created the promise (by doing +new node.Promise()+) then call +emitSuccess+ to emit the +"success"+ event with the given arguments. @@ -237,20 +233,10 @@ the moment due to a bug; use +emitSuccess+ instead.) +promise.emitError(arg1, arg2, ...)+ :: Emits the +"error"+ event. -+promise.emitCancel(arg1, arg2, ...)+ :: -Emits the +"cancel"+ event. You may still get a +"success"+ or +"error"+ -callback if the promise giver does not handle the cancel event. Use -+promise.cancel()+ to ignore any later events. - -+promise.cancel()+ :: -Clears all +"success"+ and +"error"+ event listeners from the promise, then -emits the +"cancel"+ event. Whether or not the promise is actually canceled -or not depends on the promise giver. - +promise.timeout(timeout = undefined)+ :: If the +timeout+ parameter is provided, the promise will emit an +"error"+ event after the given amount of millseconds. The timeout is canceled by any -+"success"+, +"error"+ or +"cancel"+ event being emitted by the Promise. ++"success"+ or +"error"+ event being emitted by the Promise. + To tell apart a timeout from a regular "error" event, use the following test: + diff --git a/src/events.js b/src/events.js index 789a06454..e145649dc 100644 --- a/src/events.js +++ b/src/events.js @@ -20,24 +20,6 @@ node.EventEmitter.prototype.listeners = function (type) { return this._events[type]; }; -// node.Promise is defined in src/events.cc -node.Promise.prototype.cancel = function() { - this._events['success'] = []; - this._events['error'] = []; - - this.emitSuccess = function() {}; - this.emitError = function() {}; - - this.emitCancel(); -}; - -node.Promise.prototype.emitCancel = function() { - var args = Array.prototype.slice.call(arguments); - args.unshift('cancel'); - - this.emit.apply(this, args); -}; - node.Promise.prototype.timeout = function(timeout) { if (timeout === undefined) { return this._timeoutDuration; @@ -51,7 +33,6 @@ node.Promise.prototype.timeout = function(timeout) { var self = this this._timer = setTimeout(function() { self.emitError(new Error('timeout')); - self.cancel(); }, this._timeoutDuration); return this; @@ -67,11 +48,6 @@ node.Promise.prototype.addErrback = function (listener) { return this; }; -node.Promise.prototype.addCancelback = function (listener) { - this.addListener("cancel", listener); - return this; -}; - node.Promise.prototype.wait = function () { var ret; var had_error = false; diff --git a/src/node_events.cc b/src/node_events.cc index bff32abdf..6e0d5de3f 100644 --- a/src/node_events.cc +++ b/src/node_events.cc @@ -205,16 +205,22 @@ void Promise::Detach(void) { } bool Promise::EmitSuccess(int argc, v8::Handle argv[]) { + if (has_fired_) return false; + bool r = Emit("success", argc, argv); + has_fired_ = true; Detach(); return r; } bool Promise::EmitError(int argc, v8::Handle argv[]) { + if (has_fired_) return false; + bool r = Emit("error", argc, argv); + has_fired_ = true; Detach(); return r; diff --git a/src/node_events.h b/src/node_events.h index d6e311127..23433a543 100644 --- a/src/node_events.h +++ b/src/node_events.h @@ -43,12 +43,14 @@ class Promise : public EventEmitter { virtual void Detach(void); + bool has_fired_; bool blocking_; Promise *prev_; /* for the prev in the Poor Man's coroutine stack */ void Destack(); Promise() : EventEmitter() { + has_fired_ = false; blocking_ = false; prev_ = NULL; } diff --git a/test/mjsunit/test-promise-cancel.js b/test/mjsunit/test-promise-cancel.js deleted file mode 100644 index a3184b2ed..000000000 --- a/test/mjsunit/test-promise-cancel.js +++ /dev/null @@ -1,27 +0,0 @@ -node.mixin(require("common.js")); - -var cancelFired = false; - -var promise = new node.Promise(); -promise.addCallback(function() { - assertUnreachable('addCallback should not fire after promise.cancel()'); -}); - -promise.addErrback(function() { - assertUnreachable('addErrback should not fire after promise.cancel()'); -}); - -promise.addCancelback(function() { - cancelFired = true; -}); - -promise.cancel(); - -setTimeout(function() { - promise.emitSuccess(); - promise.emitError(); -}, 100); - -process.addListener('exit', function() { - assertTrue(cancelFired); -}); \ No newline at end of file