node: change AsyncListener API

There was a flaw in the old API that has been fixed. Now the
asyncListener callback is now the "create" object property in the
callback object, and is optional.

doc/api/process.markdown

@@ -688,68 +688,68 @@ a diff reading, useful for benchmarks and measuring intervals:
 The `AsyncListener` API is the JavaScript interface for the `AsyncWrap`
 class which allows developers to be notified about key events in the
 lifetime of an asynchronous event. Node performs a lot of asynchronous
-events internally, and significant use of this API will have a **dramatic
-performance impact** on your application.
+events internally, and significant use of this API may have a
+**significant performance impact** on your application.
 
 
-## process.createAsyncListener(asyncListener[, callbacksObj[, storageValue]])
+## process.createAsyncListener(callbacksObj[, userData])
 
-* `asyncListener` {Function} callback fired when an asynchronous event is
-instantiated.
-* `callbacksObj` {Object} optional callbacks that will fire at specific
-times in the lifetime of the asynchronous event.
-* `storageValue` {Value} a value that will be passed as the first argument
-when the `asyncListener` callback is run, and to all subsequent callback.
+* `callbacksObj` {Object} Contains optional callbacks that will fire at
+specific times in the life cycle of the asynchronous event.
+* `userData` {Value} a value that will be passed to all callbacks.
 
 Returns a constructed `AsyncListener` object.
 
-To begin capturing asynchronous events pass the object to
-[`process.addAsyncListener()`][]. The same `AsyncListener` instance can
-only be added once to the active queue, and subsequent attempts to add the
-instance will be ignored.
+To begin capturing asynchronous events pass either the `callbacksObj` or
+and existing `AsyncListener` instance to [`process.addAsyncListener()`][].
+The same `AsyncListener` instance can only be added once to the active
+queue, and subsequent attempts to add the instance will be ignored.
 
-To stop capturing pass the object to [`process.removeAsyncListener()`][].
-This does _not_ mean the `AsyncListener` previously added will stop
-triggering callbacks. Once attached to an asynchronous event it will
-persist with the lifetime of the asynchronous call stack.
+To stop capturing pass the `AsyncListener` instance to
+[`process.removeAsyncListener()`][]. This does _not_ mean the
+`AsyncListener` previously added will stop triggering callbacks. Once
+attached to an asynchronous event it will persist with the lifetime of the
+asynchronous call stack.
 
 Explanation of function parameters:
 
-`asyncListener(storageValue)`: A `Function` called when an asynchronous
+
+`callbacksObj`: An `Object` which may contain three optional fields:
+
+* `create(userData)`: A `Function` called when an asynchronous
 event is instantiated. If a `Value` is returned then it will be attached
 to the event and overwrite any value that had been passed to
-`process.createAsyncListener()`'s `storageValue` argument. If an initial
-`storageValue` was passed when created, then `asyncListener()` will
+`process.createAsyncListener()`'s `userData` argument. If an initial
+`userData` was passed when created, then `create()` will
 receive that as a function argument.
 
-`callbacksObj`: An `Object` which may contain three optional fields:
-
-* `before(context, storageValue)`: A `Function` that is called immediately
+* `before(context, userData)`: A `Function` that is called immediately
 before the asynchronous callback is about to run. It will be passed both
-the `context` (i.e. `this`) of the calling function and the `storageValue`
-either returned from `asyncListener` or passed during construction (if
+the `context` (i.e. `this`) of the calling function and the `userData`
+either returned from `create()` or passed during construction (if
 either occurred).
 
-* `after(context, storageValue)`: A `Function` called immediately after
+* `after(context, userData)`: A `Function` called immediately after
 the asynchronous event's callback has run. Note this will not be called
 if the callback throws and the error is not handled.
 
-* `error(storageValue, error)`: A `Function` called if the event's
-callback threw. If `error` returns `true` then Node will assume the error
-has been properly handled and resume execution normally. When multiple
-`error()` callbacks have been registered, only **one** of those callbacks
-needs to return `true` for `AsyncListener` to accept that the error has
-been handled.
+* `error(userData, error)`: A `Function` called if the event's
+callback threw. If this registered callback returns `true` then Node will
+assume the error has been properly handled and resume execution normally.
+When multiple `error()` callbacks have been registered only **one** of
+those callbacks needs to return `true` for `AsyncListener` to accept that
+the error has been handled, but all `error()` callbacks will always be run.
 
-`storageValue`: A `Value` (i.e. anything) that will be, by default,
+`userData`: A `Value` (i.e. anything) that will be, by default,
 attached to all new event instances. This will be overwritten if a `Value`
-is returned by `asyncListener()`.
+is returned by `create()`.
 
-Here is an example of overwriting the `storageValue`:
+Here is an example of overwriting the `userData`:
 
-    process.createAsyncListener(function listener(value) {
-      // value === true
-      return false;
+    process.createAsyncListener({
+      create: function listener(value) {
+        // value === true
+        return false;
     }, {
       before: function before(context, value) {
         // value === false
@@ -757,12 +757,12 @@ Here is an example of overwriting the `storageValue`:
     }, true);
 
 **Note:** The [EventEmitter][], while used to emit status of an asynchronous
-event, is not itself asynchronous. So `asyncListener()` will not fire when
+event, is not itself asynchronous. So `create()` will not fire when
 an event is added, and `before`/`after` will not fire when emitted
 callbacks are called.
 
 
-## process.addAsyncListener(asyncListener[, callbacksObj[, storageValue]])
+## process.addAsyncListener(callbacksObj[, userData])
 ## process.addAsyncListener(asyncListener)
 
 Returns a constructed `AsyncListener` object and immediately adds it to
@@ -774,36 +774,32 @@ object.
 
 Example usage for capturing errors:
 
+    var fs = require('fs');
+
     var cntr = 0;
-    var key = process.addAsyncListener(function() {
-      return { uid: cntr++ };
-    }, {
+    var key = process.addAsyncListener({
+      create: function onCreate() {
+        return { uid: cntr++ };
+      },
       before: function onBefore(context, storage) {
-        // Need to remove the listener while logging or will end up
-        // with an infinite call loop.
-        process.removeAsyncListener(key);
-        console.log('uid: %s is about to run', storage.uid);
-        process.addAsyncListener(key);
+        // Write directly to stdout or we'll enter a recursive loop
+        fs.writeSync(1, 'uid: ' + storage.uid + ' is about to run\n');
       },
       after: function onAfter(context, storage) {
-        process.removeAsyncListener(key);
-        console.log('uid: %s is about to run', storage.uid);
-        process.addAsyncListener(key);
+        fs.writeSync(1, 'uid: ' + storage.uid + ' is about to run\n');
       },
       error: function onError(storage, err) {
         // Handle known errors
-        if (err.message === 'really, it\'s ok') {
-          process.removeAsyncListener(key);
-          console.log('handled error just threw:');
-          console.log(err.stack);
-          process.addAsyncListener(key);
+        if (err.message === 'everything is fine') {
+          fs.writeSync(1, 'handled error just threw:\n');
+          fs.writeSync(1, err.stack + '\n');
           return true;
         }
       }
     });
 
     process.nextTick(function() {
-      throw new Error('really, it\'s ok');
+      throw new Error('everything is fine');
     });
 
     // Output:
@@ -820,16 +816,19 @@ Example usage for capturing errors:
 
 Removes the `AsyncListener` from the listening queue.
 
-Removing the `AsyncListener` from the queue does _not_ mean asynchronous
-events called during its execution scope will stop firing callbacks. Once
-attached to an event it will persist for the entire asynchronous call
-stack. For example:
+Removing the `AsyncListener` from the active queue does _not_ mean the
+`asyncListener` callbacks will cease to fire on the events they've been
+registered. Subsequently, any asynchronous events fired during the
+execution of a callback will also have the same `asyncListener` callbacks
+attached for future execution. For example:
 
-    var key = process.createAsyncListener(function asyncListener() {
-      // To log we must stop listening or we'll enter infinite recursion.
-      process.removeAsyncListener(key);
-      console.log('You summoned me?');
-      process.addAsyncListener(key);
+    var fs = require('fs');
+
+    var key = process.createAsyncListener({
+      create: function asyncListener() {
+        // Write directly to stdout or we'll enter a recursive loop
+        fs.writeSync(1, 'You summoned me?\n');
+      }
     });
 
     // We want to begin capturing async events some time in the future.
@@ -861,11 +860,13 @@ To stop capturing from a specific asynchronous event stack
 `process.removeAsyncListener()` must be called from within the call
 stack itself. For example:
 
-    var key = process.createAsyncListener(function asyncListener() {
-      // To log we must stop listening or we'll enter infinite recursion.
-      process.removeAsyncListener(key);
-      console.log('You summoned me?');
-      process.addAsyncListener(key);
+    var fs = require('fs');
+
+    var key = process.createAsyncListener({
+      create: function asyncListener() {
+        // Write directly to stdout or we'll enter a recursive loop
+        fs.writeSync(1, 'You summoned me?\n');
+      }
     });
 
     // We want to begin capturing async events some time in the future.

lib/domain.js

@@ -42,9 +42,6 @@ exports._stack = stack;
 exports.active = null;
 
 
-function noop() { }
-
-
 var listenerObj = {
   error: function errorHandler(domain, er) {
     var caught = false;
@@ -104,7 +101,7 @@ inherits(Domain, EventEmitter);
 function Domain() {
   EventEmitter.call(this);
   this.members = [];
-  this._listener = process.createAsyncListener(noop, listenerObj, this);
+  this._listener = process.createAsyncListener(listenerObj, this);
 }
 
 Domain.prototype.members = undefined;

src/node.js

@@ -311,18 +311,21 @@
       inAsyncTick = true;
       for (i = 0; i < asyncQueue.length; i++) {
         queueItem = asyncQueue[i];
+        if (!queueItem.callbacks.create) {
+          queue[i] = queueItem;
+          continue;
+        }
         // Not passing "this" context because it hasn't actually been
         // instantiated yet, so accessing some of the object properties
         // can cause a segfault.
         // Passing the original value will allow users to manipulate the
         // original value object, while also allowing them to return a
         // new value for current async call tracking.
-        value = queueItem.listener(queueItem.value);
+        value = queueItem.callbacks.create(queueItem.value);
         if (typeof value !== 'undefined') {
           item = {
             callbacks: queueItem.callbacks,
             value: value,
-            listener: queueItem.listener,
             uid: queueItem.uid
           };
         } else {
@@ -388,22 +391,19 @@
 
     // Create new async listener object. Useful when instantiating a new
     // object and want the listener instance, but not add it to the stack.
-    function createAsyncListener(listener, callbacks, value) {
+    function createAsyncListener(callbacks, value) {
       return {
         callbacks: callbacks,
         value: value,
-        listener: listener,
         uid: uid++
       };
     }
 
     // Add a listener to the current queue.
-    function addAsyncListener(listener, callbacks, value) {
+    function addAsyncListener(callbacks, value) {
       // Accept new listeners or previous created listeners.
-      if (typeof listener === 'function')
-        callbacks = createAsyncListener(listener, callbacks, value);
-      else
-        callbacks = listener;
+      if (typeof callbacks.uid !== 'number')
+        callbacks = createAsyncListener(callbacks, value);
 
       var inQueue = false;
       // The asyncQueue will be small. Probably always <= 3 items.

test/simple/test-asynclistener-error-add-after.js

@@ -30,8 +30,6 @@ var caught = 0;
 var expectCaught = 0;
 var exitCbRan = false;
 
-function asyncL() { }
-
 var callbacksObj = {
   error: function(value, er) {
     var idx = errorMsgs.indexOf(er.message);
@@ -48,7 +46,7 @@ var callbacksObj = {
   }
 };
 
-var listener = process.createAsyncListener(asyncL, callbacksObj);
+var listener = process.createAsyncListener(callbacksObj);
 
 process.on('exit', function(code) {
   // Just in case.

test/simple/test-asynclistener-error-multiple-handled.js

@@ -33,17 +33,24 @@ function onAsync1() {
   return 1;
 }
 
+function onError(stor) {
+  results.push(stor);
+  return true;
+}
+
 var results = [];
-var asyncNoHandleError = {
-  error: function(stor) {
-    results.push(stor);
-    return true;
-  }
+var asyncNoHandleError0 = {
+  create: onAsync0,
+  error: onError
+};
+var asyncNoHandleError1 = {
+  create: onAsync1,
+  error: onError
 };
 
 var listeners = [
-  process.addAsyncListener(onAsync0, asyncNoHandleError),
-  process.addAsyncListener(onAsync1, asyncNoHandleError)
+  process.addAsyncListener(asyncNoHandleError0),
+  process.addAsyncListener(asyncNoHandleError1)
 ];
 
 process.nextTick(function() {

test/simple/test-asynclistener-error-multiple-mix.js

@@ -22,8 +22,6 @@
 var common = require('../common');
 var assert = require('assert');
 
-function onAsync() {}
-
 var results = [];
 var asyncNoHandleError = {
   error: function(stor) {
@@ -39,8 +37,8 @@ var asyncHandleError = {
 };
 
 var listeners = [
-  process.addAsyncListener(onAsync, asyncHandleError),
-  process.addAsyncListener(onAsync, asyncNoHandleError)
+  process.addAsyncListener(asyncHandleError),
+  process.addAsyncListener(asyncNoHandleError)
 ];
 
 // Even if an error handler returns true, both should fire.

test/simple/test-asynclistener-error-multiple-unhandled.js

@@ -30,16 +30,23 @@ function onAsync1() {
   return 1;
 }
 
+function onError(stor) {
+  results.push(stor);
+}
+
 var results = [];
-var asyncNoHandleError = {
-  error: function(stor) {
-    results.push(stor);
-  }
+var asyncNoHandleError0 = {
+  create: onAsync0,
+  error: onError
+};
+var asyncNoHandleError1 = {
+  create: onAsync1,
+  error: onError
 };
 
 var listeners = [
-  process.addAsyncListener(onAsync0, asyncNoHandleError),
-  process.addAsyncListener(onAsync1, asyncNoHandleError)
+  process.addAsyncListener(asyncNoHandleError0),
+  process.addAsyncListener(asyncNoHandleError1)
 ];
 
 var uncaughtFired = false;

test/simple/test-asynclistener-error-net.js

@@ -29,8 +29,6 @@ var errorMsgs = [];
 var caught = 0;
 var expectCaught = 0;
 
-function asyncL() { }
-
 var callbacksObj = {
   error: function(value, er) {
     var idx = errorMsgs.indexOf(er.message);
@@ -47,7 +45,7 @@ var callbacksObj = {
   }
 };
 
-var listener = process.addAsyncListener(asyncL, callbacksObj);
+var listener = process.addAsyncListener(callbacksObj);
 
 process.on('exit', function(code) {
   process.removeAsyncListener(listener);

test/simple/test-asynclistener-error-throw-in-after.js

@@ -23,7 +23,6 @@ var common = require('../common');
 var assert = require('assert');
 
 var once = 0;
-function onAsync0() { }
 
 var results = [];
 var handlers = {
@@ -38,7 +37,7 @@ var handlers = {
   }
 }
 
-var key = process.addAsyncListener(onAsync0, handlers);
+var key = process.addAsyncListener(handlers);
 
 var uncaughtFired = false;
 process.on('uncaughtException', function(err) {

test/simple/test-asynclistener-error-throw-in-before-multiple.js

@@ -23,8 +23,6 @@ var common = require('../common');
 var assert = require('assert');
 
 var once = 0;
-function onAsync0() { }
-function onAsync1() { }
 
 var results = [];
 var handlers = {
@@ -52,8 +50,8 @@ var handlers1 = {
 }
 
 var listeners = [
-  process.addAsyncListener(onAsync0, handlers),
-  process.addAsyncListener(onAsync1, handlers1)
+  process.addAsyncListener(handlers),
+  process.addAsyncListener(handlers1)
 ];
 
 var uncaughtFired = false;

test/simple/test-asynclistener-error-throw-in-before.js

@@ -23,7 +23,6 @@ var common = require('../common');
 var assert = require('assert');
 
 var once = 0;
-function onAsync0() {}
 
 var results = [];
 var handlers = {
@@ -38,7 +37,7 @@ var handlers = {
   }
 }
 
-var key = process.addAsyncListener(onAsync0, handlers);
+var key = process.addAsyncListener(handlers);
 
 var uncaughtFired = false;
 process.on('uncaughtException', function(err) {

test/simple/test-asynclistener-error-throw-in-error.js

@@ -34,7 +34,7 @@ else
 function runChild() {
   var cntr = 0;
 
-  var key = process.addAsyncListener(function() { }, {
+  var key = process.addAsyncListener({
     error: function onError() {
       cntr++;
       throw new Error('onError');

test/simple/test-asynclistener-error.js

@@ -33,8 +33,6 @@ var caught = 0;
 var expectCaught = 0;
 var exitCbRan = false;
 
-function asyncL() { }
-
 var callbacksObj = {
   error: function(value, er) {
     var idx = errorMsgs.indexOf(er.message);
@@ -48,7 +46,7 @@ var callbacksObj = {
   }
 };
 
-var listener = process.createAsyncListener(asyncL, callbacksObj);
+var listener = process.createAsyncListener(callbacksObj);
 
 process.on('exit', function(code) {
   removeListener(listener);

test/simple/test-asynclistener-multi-timeout.js

@@ -27,8 +27,6 @@ var removeListener = process.removeAsyncListener;
 var caught = [];
 var expect = [];
 
-function asyncL(a) {}
-
 var callbacksObj = {
   error: function(value, er) {
     process._rawDebug('caught', er.message);
@@ -37,7 +35,7 @@ var callbacksObj = {
   }
 };
 
-var listener = process.createAsyncListener(asyncL, callbacksObj);
+var listener = process.createAsyncListener(callbacksObj);
 
 process.on('exit', function(code) {
   removeListener(listener);

test/simple/test-asynclistener-remove-add-in-before.js

@@ -23,6 +23,9 @@ var common = require('../common');
 var assert = require('assert');
 var val;
 var callbacks = {
+  create: function() {
+    return 42;
+  },
   before: function() {
     process.removeAsyncListener(listener);
     process.addAsyncListener(listener);
@@ -32,14 +35,12 @@ var callbacks = {
   }
 };
 
-var listener = process.addAsyncListener(function() {
-  return 66;
-}, callbacks);
+var listener = process.addAsyncListener(callbacks);
 
 process.nextTick(function() {});
 
 process.on('exit', function(status) {
   process.removeAsyncListener(listener);
   assert.equal(status, 0);
-  assert.equal(val, 66);
+  assert.equal(val, 42);
 });

test/simple/test-asynclistener-remove-after.js

@@ -27,7 +27,7 @@ var net = require('net');
 // TODO(trevnorris): Test has the flaw that it's not checking if the async
 // flag has been removed on the class instance. Though currently there's
 // no way to do that.
-var listener = process.addAsyncListener(function() { });
+var listener = process.addAsyncListener({ create: function() { }});
 
 
 // Test timers

test/simple/test-asynclistener-remove-before.js

@@ -23,8 +23,6 @@ var common = require('../common');
 var assert = require('assert');
 var set = 0;
 
-function onAsync0() { }
-
 var asyncNoHandleError = {
   before: function() {
     set++;
@@ -34,7 +32,7 @@ var asyncNoHandleError = {
   }
 }
 
-var key = process.addAsyncListener(onAsync0, asyncNoHandleError);
+var key = process.addAsyncListener(asyncNoHandleError);
 
 process.removeAsyncListener(key);
 

test/simple/test-asynclistener-remove-in-before.js

@@ -31,7 +31,7 @@ var callbacks = {
   }
 };
 
-var listener = process.addAsyncListener(function() {}, callbacks);
+var listener = process.addAsyncListener(callbacks);
 
 process.nextTick(function() {});
 

test/simple/test-asynclistener-remove-inflight-error.js

@@ -22,8 +22,6 @@
 var common = require('../common');
 var assert = require('assert');
 
-function onAsync0() { }
-
 var set = 0;
 var asyncNoHandleError = {
   error: function() {
@@ -31,7 +29,7 @@ var asyncNoHandleError = {
   }
 }
 
-var key = process.addAsyncListener(onAsync0, asyncNoHandleError);
+var key = process.addAsyncListener(asyncNoHandleError);
 
 process.nextTick(function() {
   throw 1;

test/simple/test-asynclistener-remove-inflight.js

@@ -22,8 +22,6 @@
 var common = require('../common');
 var assert = require('assert');
 
-function onAsync0() { }
-
 var set = 0;
 var asyncNoHandleError = {
   before: function() {
@@ -34,7 +32,7 @@ var asyncNoHandleError = {
   }
 }
 
-var key = process.addAsyncListener(onAsync0, asyncNoHandleError);
+var key = process.addAsyncListener(asyncNoHandleError);
 
 process.nextTick(function() { });
 

test/simple/test-asynclistener-throw-before-infinite-recursion.js

@@ -31,7 +31,7 @@ var assert = require('assert');
 var cntr = 0;
 
 
-process.addAsyncListener(function() { }, {
+process.addAsyncListener({
   before: function() {
     if (++cntr > 1) {
       // Can't throw since uncaughtException will also catch that.

test/simple/test-asynclistener.js

@@ -30,11 +30,13 @@ var removeListener = process.removeAsyncListener;
 var actualAsync = 0;
 var expectAsync = 0;
 
-function onAsync() {
-  actualAsync++;
-}
+var callbacks = {
+  create: function onAsync() {
+    actualAsync++;
+  }
+};
 
-var listener = process.createAsyncListener(onAsync);
+var listener = process.createAsyncListener(callbacks);
 
 process.on('exit', function() {
   process._rawDebug('expected', expectAsync);