From 7b5e8126049fa3fec9fb951ef7f2062d417c0e81 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Sat, 9 Jan 2016 10:58:55 +0100 Subject: [PATCH 1/3] async_wrap: add uid to all asyncWrap hooks By doing this users can use a Map object for storing information instead of modifying the handle object. --- src/async-wrap.cc | 5 ++- test/parallel/test-async-wrap-uid.js | 57 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-async-wrap-uid.js diff --git a/src/async-wrap.cc b/src/async-wrap.cc index a7a9822238329a..11a696cc2329a3 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -179,6 +179,7 @@ Local AsyncWrap::MakeCallback(const Local cb, Local pre_fn = env()->async_hooks_pre_function(); Local post_fn = env()->async_hooks_post_function(); + Local uid = Integer::New(env()->isolate(), get_uid()); Local context = object(); Local process = env()->process_object(); Local domain; @@ -207,14 +208,14 @@ Local AsyncWrap::MakeCallback(const Local cb, } if (ran_init_callback() && !pre_fn.IsEmpty()) { - if (pre_fn->Call(context, 0, nullptr).IsEmpty()) + if (pre_fn->Call(context, 1, &uid).IsEmpty()) FatalError("node::AsyncWrap::MakeCallback", "pre hook threw"); } Local ret = cb->Call(context, argc, argv); if (ran_init_callback() && !post_fn.IsEmpty()) { - if (post_fn->Call(context, 0, nullptr).IsEmpty()) + if (post_fn->Call(context, 1, &uid).IsEmpty()) FatalError("node::AsyncWrap::MakeCallback", "post hook threw"); } diff --git a/test/parallel/test-async-wrap-uid.js b/test/parallel/test-async-wrap-uid.js new file mode 100644 index 00000000000000..ad2dd5027cfa1d --- /dev/null +++ b/test/parallel/test-async-wrap-uid.js @@ -0,0 +1,57 @@ +'use strict'; + +require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const async_wrap = process.binding('async_wrap'); + +const storage = new Map(); +async_wrap.setupHooks(init, pre, post, destroy); +async_wrap.enable(); + +function init(provider, uid) { + storage.set(uid, { + init: true, + pre: false, + post: false, + destroy: false + }); +} + +function pre(uid) { + storage.get(uid).pre = true; +} + +function post(uid) { + storage.get(uid).post = true; +} + +function destroy(uid) { + storage.get(uid).destroy = true; +} + +fs.access(__filename, function(err) { + assert.ifError(err); +}); + +fs.access(__filename, function(err) { + assert.ifError(err); +}); + +async_wrap.disable(); + +process.once('exit', function() { + assert.strictEqual(storage.size, 2); + + for (const item of storage) { + const uid = item[0]; + const value = item[1]; + assert.strictEqual(typeof uid, 'number'); + assert.deepStrictEqual(value, { + init: true, + pre: true, + post: true, + destroy: true + }); + } +}); From 394f170ef341d8ab12710797ce5cb04cd54f1323 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Sat, 9 Jan 2016 12:27:53 +0100 Subject: [PATCH 2/3] async_wrap: make uid the first argument in init All other hooks have uid as the first argument, this makes it concistent for all hooks. --- src/async-wrap-inl.h | 2 +- test/parallel/test-async-wrap-check-providers.js | 4 ++-- test/parallel/test-async-wrap-disabled-propagate-parent.js | 2 +- test/parallel/test-async-wrap-propagate-parent.js | 2 +- test/parallel/test-async-wrap-uid.js | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index 1d9ebe27e45bef..e68c6af4d6efd1 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -40,8 +40,8 @@ inline AsyncWrap::AsyncWrap(Environment* env, v8::HandleScope scope(env->isolate()); v8::Local argv[] = { - v8::Int32::New(env->isolate(), provider), v8::Integer::New(env->isolate(), get_uid()), + v8::Int32::New(env->isolate(), provider), Null(env->isolate()) }; diff --git a/test/parallel/test-async-wrap-check-providers.js b/test/parallel/test-async-wrap-check-providers.js index 38302be2cd0b5d..966d0b74a26cce 100644 --- a/test/parallel/test-async-wrap-check-providers.js +++ b/test/parallel/test-async-wrap-check-providers.js @@ -19,8 +19,8 @@ let keyList = pkeys.slice(); keyList.splice(0, 1); -function init(id) { - keyList = keyList.filter((e) => e != pkeys[id]); +function init(id, provider) { + keyList = keyList.filter((e) => e != pkeys[provider]); } function noop() { } diff --git a/test/parallel/test-async-wrap-disabled-propagate-parent.js b/test/parallel/test-async-wrap-disabled-propagate-parent.js index 70d82befe72a63..0b987a5495e66c 100644 --- a/test/parallel/test-async-wrap-disabled-propagate-parent.js +++ b/test/parallel/test-async-wrap-disabled-propagate-parent.js @@ -10,7 +10,7 @@ let cntr = 0; let server; let client; -function init(type, id, parent) { +function init(id, type, parent) { if (parent) { cntr++; // Cannot assert in init callback or will abort. diff --git a/test/parallel/test-async-wrap-propagate-parent.js b/test/parallel/test-async-wrap-propagate-parent.js index beeb27ba7866a9..99322b03f7adfd 100644 --- a/test/parallel/test-async-wrap-propagate-parent.js +++ b/test/parallel/test-async-wrap-propagate-parent.js @@ -9,7 +9,7 @@ let cntr = 0; let server; let client; -function init(type, id, parent) { +function init(id, type, parent) { if (parent) { cntr++; // Cannot assert in init callback or will abort. diff --git a/test/parallel/test-async-wrap-uid.js b/test/parallel/test-async-wrap-uid.js index ad2dd5027cfa1d..4e664f1bd46ebb 100644 --- a/test/parallel/test-async-wrap-uid.js +++ b/test/parallel/test-async-wrap-uid.js @@ -9,7 +9,7 @@ const storage = new Map(); async_wrap.setupHooks(init, pre, post, destroy); async_wrap.enable(); -function init(provider, uid) { +function init(uid) { storage.set(uid, { init: true, pre: false, From 8bfc6f38dd1d4f1150e5e81958f82346f8f15aed Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Wed, 20 Jan 2016 18:20:43 +0100 Subject: [PATCH 3/3] async_wrap: add parent uid to init hook When the parent uid is required it is not necessary to store the uid in the parent handle object. --- src/async-wrap-inl.h | 7 +++++-- .../test-async-wrap-disabled-propagate-parent.js | 13 ++++++++++--- test/parallel/test-async-wrap-propagate-parent.js | 15 ++++++++++++--- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index e68c6af4d6efd1..e6b24af7fd731f 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -42,11 +42,14 @@ inline AsyncWrap::AsyncWrap(Environment* env, v8::Local argv[] = { v8::Integer::New(env->isolate(), get_uid()), v8::Int32::New(env->isolate(), provider), + Null(env->isolate()), Null(env->isolate()) }; - if (parent != nullptr) - argv[2] = parent->object(); + if (parent != nullptr) { + argv[2] = v8::Integer::New(env->isolate(), parent->get_uid()); + argv[3] = parent->object(); + } v8::MaybeLocal ret = init_fn->Call(env->context(), object, ARRAY_SIZE(argv), argv); diff --git a/test/parallel/test-async-wrap-disabled-propagate-parent.js b/test/parallel/test-async-wrap-disabled-propagate-parent.js index 0b987a5495e66c..ee674c43ffe6b1 100644 --- a/test/parallel/test-async-wrap-disabled-propagate-parent.js +++ b/test/parallel/test-async-wrap-disabled-propagate-parent.js @@ -6,17 +6,24 @@ const net = require('net'); const async_wrap = process.binding('async_wrap'); const providers = Object.keys(async_wrap.Providers); +const uidSymbol = Symbol('uid'); + let cntr = 0; let server; let client; -function init(id, type, parent) { - if (parent) { +function init(uid, type, parentUid, parentHandle) { + this[uidSymbol] = uid; + + if (parentHandle) { cntr++; // Cannot assert in init callback or will abort. process.nextTick(() => { assert.equal(providers[type], 'TCPWRAP'); - assert.equal(parent, server._handle, 'server doesn\'t match parent'); + assert.equal(parentUid, server._handle[uidSymbol], + 'server uid doesn\'t match parent uid'); + assert.equal(parentHandle, server._handle, + 'server handle doesn\'t match parent handle'); assert.equal(this, client._handle, 'client doesn\'t match context'); }); } diff --git a/test/parallel/test-async-wrap-propagate-parent.js b/test/parallel/test-async-wrap-propagate-parent.js index 99322b03f7adfd..c27803832df6fe 100644 --- a/test/parallel/test-async-wrap-propagate-parent.js +++ b/test/parallel/test-async-wrap-propagate-parent.js @@ -4,17 +4,26 @@ const common = require('../common'); const assert = require('assert'); const net = require('net'); const async_wrap = process.binding('async_wrap'); +const providers = Object.keys(async_wrap.Providers); + +const uidSymbol = Symbol('uid'); let cntr = 0; let server; let client; -function init(id, type, parent) { - if (parent) { +function init(uid, type, parentUid, parentHandle) { + this[uidSymbol] = uid; + + if (parentHandle) { cntr++; // Cannot assert in init callback or will abort. process.nextTick(() => { - assert.equal(parent, server._handle, 'server doesn\'t match parent'); + assert.equal(providers[type], 'TCPWRAP'); + assert.equal(parentUid, server._handle[uidSymbol], + 'server uid doesn\'t match parent uid'); + assert.equal(parentHandle, server._handle, + 'server handle doesn\'t match parent handle'); assert.equal(this, client._handle, 'client doesn\'t match context'); }); }