-
Notifications
You must be signed in to change notification settings - Fork 30.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
async_wrap: add uid argument to all asyncWrap hooks #4600
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
'use strict'; | ||
|
||
require('../common'); | ||
const fs = require('fs'); | ||
const assert = require('assert'); | ||
const async_wrap = process.binding('async_wrap'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In JavaScript code we follow camelCase style. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know, but this is consistent with the other |
||
|
||
const storage = new Map(); | ||
async_wrap.setupHooks(init, pre, post, destroy); | ||
async_wrap.enable(); | ||
|
||
function init(uid) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about asserting if we got all four parameters? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool then 👍 |
||
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 | ||
}); | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the meaning of the number 1 here? Also, will the uid be enough during pre or post calls?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It specifies the length of the arguments array (
&uid
) that follows.The general idea is that the user will collect all the required information in the init hook and map it to a
Map
object using theuid
. Thus theuid
should be all that the user needs in thepre
,post
anddestroy
hook.