From 63ccfc35366774fff1ee8db313a1b9ea6d6eb66c Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Mon, 20 Jan 2014 13:20:11 -0800 Subject: [PATCH] async_wrap/timers: remove Add/RemoveAsyncListener The ability to add/remove an AsyncListener to an object after its creation was an artifact of trying to get AL working with the domain module. Now that is no longer necessary and other features are going to be implemented that would be affected by this functionality. So the code will be removed for now to simplify the implementation process. In the future this code will likely be reintroduced, but after some other more important matters have been addressed. None of this functionality was documented, as is was meant specifically for domain specific implementation work arounds. Signed-off-by: Timothy J Fontaine --- lib/timers.js | 57 -------- src/async-wrap-inl.h | 64 --------- src/async-wrap.h | 17 --- src/env.h | 4 +- src/node.cc | 4 - src/node.js | 57 +------- src/pipe_wrap.cc | 2 - src/tcp_wrap.cc | 2 - src/udp_wrap.cc | 3 - test/simple/test-asynclistener-error-add-after.js | 159 ---------------------- test/simple/test-asynclistener-pushlistener.js | 52 ------- test/simple/test-asynclistener-remove-after.js | 66 --------- 12 files changed, 2 insertions(+), 485 deletions(-) delete mode 100644 test/simple/test-asynclistener-error-add-after.js delete mode 100644 test/simple/test-asynclistener-pushlistener.js delete mode 100644 test/simple/test-asynclistener-remove-after.js diff --git a/lib/timers.js b/lib/timers.js index d268479..df46e8b 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -205,57 +205,6 @@ exports.active = function(item) { }; -// new Array() is used here because it is more efficient for sparse -// arrays. Please *do not* change these to simple bracket notation. -function timerAddAsyncListener(obj, data) { - obj = process.createAsyncListener(obj, data); - - if (!this._asyncQueue) - this._asyncQueue = new Array(); - if (!this._asyncData) - this._asyncData = new Array(); - - var inQueue = false; - var queue = this._asyncQueue; - // This queue will be small. Probably always <= 3 items. - for (var i = 0; i < queue.length; i++) { - if (queue[i].uid === obj.uid) { - inQueue = true; - break; - } - } - - if (!inQueue) { - queue.push(obj); - this._asyncData[obj.uid] = obj.data; - this._asyncFlags |= obj.flags; - } - - return obj; -} - - -function timerRemoveAsyncListener(obj) { - if (!this._asyncQueue) - return; - var queue = this._asyncQueue; - var i; - // This queue will be small. Probably always <= 3 items. - for (i = 0; i < queue.length; i++) { - if (queue[i].uid === obj.uid) { - queue.splice(i, 1); - this._asyncData[obj.uid] = undefined; - return; - } - } - // Rebuild flags - this._asyncFlags = 0; - for (i = 0; i < queue.length; i++) { - this._asyncFlags |= queue[i].flags; - } -} - - /* * DOM-style timers */ @@ -391,10 +340,6 @@ Timeout.prototype.close = function() { } }; -// For domain compatibility need to attach this API. -Timeout.prototype.addAsyncListener = timerAddAsyncListener; -Timeout.prototype.removeAsyncListener = timerRemoveAsyncListener; - var immediateQueue = {}; L.init(immediateQueue); @@ -452,8 +397,6 @@ function processImmediate() { function Immediate() { } -Immediate.prototype.addAsyncListener = timerAddAsyncListener; -Immediate.prototype.removeAsyncListener = timerRemoveAsyncListener; Immediate.prototype.domain = undefined; Immediate.prototype._onImmediate = undefined; Immediate.prototype._asyncQueue = undefined; diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index 3223d83..aad1dcb 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -57,32 +57,11 @@ inline AsyncWrap::~AsyncWrap() { } -template -inline void AsyncWrap::AddMethods(v8::Handle t) { - NODE_SET_PROTOTYPE_METHOD(t, - "addAsyncListener", - AddAsyncListener); - NODE_SET_PROTOTYPE_METHOD(t, - "removeAsyncListener", - RemoveAsyncListener); -} - - inline uint32_t AsyncWrap::async_flags() const { return async_flags_; } -inline void AsyncWrap::set_flag(unsigned int flag) { - async_flags_ |= flag; -} - - -inline void AsyncWrap::remove_flag(unsigned int flag) { - async_flags_ &= ~flag; -} - - inline bool AsyncWrap::has_async_listener() { return async_flags() & HAS_ASYNC_LISTENER; } @@ -263,49 +242,6 @@ inline v8::Handle AsyncWrap::MakeCallback( return MakeCallback(cb, argc, argv); } - -template -inline void AsyncWrap::AddAsyncListener( - const v8::FunctionCallbackInfo& args) { - v8::HandleScope handle_scope(args.GetIsolate()); - Environment* env = Environment::GetCurrent(args.GetIsolate()); - - v8::Local handle = args.This(); - v8::Local listener = args[0]; - assert(listener->IsObject()); - assert(handle->InternalFieldCount() > 0); - - env->async_listener_push_function()->Call(handle, 1, &listener); - - Type* wrap = static_cast( - handle->GetAlignedPointerFromInternalField(0)); - assert(wrap != NULL); - wrap->set_flag(HAS_ASYNC_LISTENER); -} - - -template -inline void AsyncWrap::RemoveAsyncListener( - const v8::FunctionCallbackInfo& args) { - v8::HandleScope handle_scope(args.GetIsolate()); - Environment* env = Environment::GetCurrent(args.GetIsolate()); - - v8::Local handle = args.This(); - v8::Local listener = args[0]; - assert(listener->IsObject()); - assert(handle->InternalFieldCount() > 0); - - v8::Local ret = - env->async_listener_strip_function()->Call(handle, 1, &listener); - - if (ret->IsFalse()) { - Type* wrap = static_cast( - handle->GetAlignedPointerFromInternalField(0)); - assert(wrap != NULL); - wrap->remove_flag(HAS_ASYNC_LISTENER); - } -} - } // namespace node #endif // SRC_ASYNC_WRAP_INL_H_ diff --git a/src/async-wrap.h b/src/async-wrap.h index 068dfa6..a58031b 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -39,15 +39,8 @@ class AsyncWrap : public BaseObject { inline ~AsyncWrap(); - template - static inline void AddMethods(v8::Handle t); - inline uint32_t async_flags() const; - inline void set_flag(unsigned int flag); - - inline void remove_flag(unsigned int flag); - inline bool has_async_listener(); // Only call these within a valid HandleScope. @@ -71,16 +64,6 @@ class AsyncWrap : public BaseObject { int argc, v8::Handle* argv); - // Add an async listener to an existing handle. - template - static inline void AddAsyncListener( - const v8::FunctionCallbackInfo& args); - - // Remove an async listener to an existing handle. - template - static inline void RemoveAsyncListener( - const v8::FunctionCallbackInfo& args); - uint32_t async_flags_; }; diff --git a/src/env.h b/src/env.h index b654ea2..e8796e7 100644 --- a/src/env.h +++ b/src/env.h @@ -136,10 +136,8 @@ namespace node { V(write_queue_size_string, "writeQueueSize") \ #define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) \ - V(async_listener_load_function, v8::Function) \ - V(async_listener_push_function, v8::Function) \ V(async_listener_run_function, v8::Function) \ - V(async_listener_strip_function, v8::Function) \ + V(async_listener_load_function, v8::Function) \ V(async_listener_unload_function, v8::Function) \ V(binding_cache_object, v8::Object) \ V(buffer_constructor_function, v8::Function) \ diff --git a/src/node.cc b/src/node.cc index f8cd15b..25283c0 100644 --- a/src/node.cc +++ b/src/node.cc @@ -854,14 +854,10 @@ void SetupAsyncListener(const FunctionCallbackInfo& args) { assert(args[1]->IsFunction()); assert(args[2]->IsFunction()); assert(args[3]->IsFunction()); - assert(args[4]->IsFunction()); - assert(args[5]->IsFunction()); env->set_async_listener_run_function(args[1].As()); env->set_async_listener_load_function(args[2].As()); env->set_async_listener_unload_function(args[3].As()); - env->set_async_listener_push_function(args[4].As()); - env->set_async_listener_strip_function(args[5].As()); Local async_listener_flag_obj = args[0].As(); Environment::AsyncListener* async_listener = env->async_listener(); diff --git a/src/node.js b/src/node.js index fd0d98a..29a3907 100644 --- a/src/node.js +++ b/src/node.js @@ -311,9 +311,7 @@ process._setupAsyncListener(asyncFlags, runAsyncQueue, loadAsyncQueue, - unloadAsyncQueue, - pushListener, - stripListener); + unloadAsyncQueue); // Load the currently executing context as the current context, and // create a new asyncQueue that can receive any added queue items @@ -606,59 +604,6 @@ (currentContext && currentContext._asyncQueue.length)) asyncFlags[kHasListener] = 1; } - - // Used by AsyncWrap::AddAsyncListener() to add an individual listener - // to the async queue. It will check the uid of the listener and only - // allow it to be added once. - function pushListener(obj) { - if (!this._asyncQueue) { - this._asyncQueue = [obj]; - this._asyncData = new Array(); - this._asyncData[obj.uid] = obj.data; - this._asyncFlags = obj.flags; - return; - } - - if (!this._asyncData) - this._asyncData = new Array(); - - var queue = this._asyncQueue; - var inQueue = false; - // The asyncQueue will be small. Probably always <= 3 items. - for (var i = 0; i < queue.length; i++) { - if (obj.uid === queue[i].uid) { - inQueue = true; - break; - } - } - - // Not in the queue so push it on and set the default storage. - if (!inQueue) { - queue.push(obj); - this._asyncData[obj.uid] = obj.data; - this._asyncFlags |= obj.flags; - } - } - - // Used by AsyncWrap::RemoveAsyncListener() to remove an individual - // listener from the async queue, and return whether there are still - // listeners in the queue. - function stripListener(obj) { - // No queue exists, so nothing to do. - if (!this._asyncQueue) - return false; - - var queue = this._asyncQueue; - - // The asyncQueue will be small. Probably always <= 3 items. - for (var i = 0; i < queue.length; i++) { - if (obj.uid === queue[i].uid) { - this._asyncData[queue[i].uid] = undefined; - queue.splice(i, 1); - return queue.length > 0; - } - } - } }; var assert; diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index d4f01eb..44f8dc9 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -111,8 +111,6 @@ void PipeWrap::Initialize(Handle target, NODE_SET_PROTOTYPE_METHOD(t, "setPendingInstances", SetPendingInstances); #endif - AsyncWrap::AddMethods(t); - target->Set(FIXED_ONE_BYTE_STRING(node_isolate, "Pipe"), t->GetFunction()); env->set_pipe_constructor_template(t); } diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index dad505d..2ff3b34 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -116,8 +116,6 @@ void TCPWrap::Initialize(Handle target, SetSimultaneousAccepts); #endif - AsyncWrap::AddMethods(t); - target->Set(FIXED_ONE_BYTE_STRING(node_isolate, "TCP"), t->GetFunction()); env->set_tcp_constructor_template(t); } diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index a563a7e..93b030f 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -119,9 +119,6 @@ void UDPWrap::Initialize(Handle target, NODE_SET_PROTOTYPE_METHOD(t, "ref", HandleWrap::Ref); NODE_SET_PROTOTYPE_METHOD(t, "unref", HandleWrap::Unref); - - AsyncWrap::AddMethods(t); - target->Set(FIXED_ONE_BYTE_STRING(node_isolate, "UDP"), t->GetFunction()); env->set_udp_constructor_function(t->GetFunction()); } diff --git a/test/simple/test-asynclistener-error-add-after.js b/test/simple/test-asynclistener-error-add-after.js deleted file mode 100644 index a8d7bf8..0000000 --- a/test/simple/test-asynclistener-error-add-after.js +++ /dev/null @@ -1,159 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -var common = require('../common'); -var assert = require('assert'); -var dns = require('dns'); -var fs = require('fs'); -var net = require('net'); - -var errorMsgs = []; -var caught = 0; -var expectCaught = 0; -var exitCbRan = false; - -var callbacksObj = { - error: function(value, er) { - var idx = errorMsgs.indexOf(er.message); - caught++; - - process._rawDebug('Handling error: ' + er.message); - - if (-1 < idx) - errorMsgs.splice(idx, 1); - else - throw new Error('Message not found: ' + er.message); - - return true; - } -}; - -var listener = process.createAsyncListener(callbacksObj); - -process.on('exit', function(code) { - // Just in case. - process.removeAsyncListener(listener); - - if (code > 0) - return; - - assert.ok(!exitCbRan); - exitCbRan = true; - - if (errorMsgs.length > 0) - throw new Error('Errors not fired: ' + errorMsgs); - - assert.equal(caught, expectCaught); - process._rawDebug('ok'); -}); - - -// Simple cases -errorMsgs.push('setTimeout - simple'); -errorMsgs.push('setImmediate - simple'); -errorMsgs.push('setInterval - simple'); -setTimeout(function() { - throw new Error('setTimeout - simple'); -}).addAsyncListener(listener); -expectCaught++; - -setImmediate(function() { - throw new Error('setImmediate - simple'); -}).addAsyncListener(listener); -expectCaught++; - -var b = setInterval(function() { - clearInterval(b); - throw new Error('setInterval - simple'); -}).addAsyncListener(listener); -expectCaught++; - - -// Deeply nested -errorMsgs.push('setInterval - nested'); -errorMsgs.push('setImmediate - nested'); -errorMsgs.push('setTimeout - nested'); -setTimeout(function() { - setImmediate(function() { - var b = setInterval(function() { - clearInterval(b); - throw new Error('setInterval - nested'); - }).addAsyncListener(listener); - expectCaught++; - throw new Error('setImmediate - nested'); - }).addAsyncListener(listener); - expectCaught++; - throw new Error('setTimeout - nested'); -}).addAsyncListener(listener); -expectCaught++; - - -// Net -var iter = 3; -for (var i = 0; i < iter; i++) { - errorMsgs.push('net - error: server connection'); - errorMsgs.push('net - error: client data'); - errorMsgs.push('net - error: server data'); -} -errorMsgs.push('net - error: server closed'); - -var server = net.createServer(function(c) { - c._handle.addAsyncListener(listener); - - c.on('data', function() { - if (0 === --iter) { - server.close(function() { - process._rawDebug('net - server closing'); - throw new Error('net - error: server closed'); - }); - expectCaught++; - } - process._rawDebug('net - connection received data'); - throw new Error('net - error: server data'); - }); - expectCaught++; - - c.end('bye'); - process._rawDebug('net - connection received'); - throw new Error('net - error: server connection'); -}); -expectCaught += iter; - -server.listen(common.PORT, function() { - // Test adding the async listener after server creation. Though it - // won't catch errors that originate synchronously from this point. - server._handle.addAsyncListener(listener); - for (var i = 0; i < iter; i++) - clientConnect(); -}); - -function clientConnect() { - var client = net.connect(common.PORT, function() { - client._handle.addAsyncListener(listener); - }); - - client.on('data', function() { - client.end('see ya'); - process._rawDebug('net - client received data'); - throw new Error('net - error: client data'); - }); - expectCaught++; -} diff --git a/test/simple/test-asynclistener-pushlistener.js b/test/simple/test-asynclistener-pushlistener.js deleted file mode 100644 index b5fb46b..0000000 --- a/test/simple/test-asynclistener-pushlistener.js +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -var common = require('../common'); -var assert = require('assert'); -var net = require('net'); - -var listener0 = process.createAsyncListener({ create: function() { } }); -var listener1 = process.createAsyncListener({ create: function() { } }); - -process.on('exit', function() { - assert.ok(listened); - console.log('ok'); -}); - -var server = net.createServer(function(c) { }); - -var listened = false; -server.listen(common.PORT, function() { - var handle = server._handle; - - handle.addAsyncListener(listener0); - handle.addAsyncListener(listener0); - handle.addAsyncListener(listener1); - handle.addAsyncListener(listener1); - - assert.equal(handle._asyncQueue.length, 2); - assert.deepEqual(handle._asyncQueue[0], listener0); - assert.deepEqual(handle._asyncQueue[1], listener1); - - listened = true; - - server.close(); -}); diff --git a/test/simple/test-asynclistener-remove-after.js b/test/simple/test-asynclistener-remove-after.js deleted file mode 100644 index c4fcb82..0000000 --- a/test/simple/test-asynclistener-remove-after.js +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -var common = require('../common'); -var assert = require('assert'); -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({ create: function() { }}); - - -// Test timers - -setImmediate(function() { - assert.equal(this._asyncQueue.length, 0); -}).removeAsyncListener(listener); - -setTimeout(function() { - assert.equal(this._asyncQueue.length, 0); -}).removeAsyncListener(listener); - -setInterval(function() { - clearInterval(this); - assert.equal(this._asyncQueue.length, 0); -}).removeAsyncListener(listener); - - -// Test net - -var server = net.createServer(function(c) { - c._handle.removeAsyncListener(listener); - assert.equal(c._handle._asyncQueue.length, 0); -}); - -server.listen(common.PORT, function() { - server._handle.removeAsyncListener(listener); - assert.equal(server._handle._asyncQueue.length, 0); - - var client = net.connect(common.PORT, function() { - client._handle.removeAsyncListener(listener); - assert.equal(client._handle._asyncQueue.length, 0); - client.end(); - server.close(); - }); -}); -- 2.7.4