Nodejs/node
1
0
Fork 0
mirror of https://github.com/nodejs/node.git synced 2024-11-21 10:59:27 +00:00
Code Issues Actions 25 Packages Projects Releases Wiki Activity

inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth

Browse Source 
This was previously calling the enable function by mistake. As a
result, when profiling using Chrome DevTools, the async hooks won't
be turned off properly after receiving Debugger.setAsyncCallStackDepth
with depth 0.

PR-URL: https://github.com/nodejs/node/pull/53473
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit is contained in:
Joyee Cheung 2024-06-18 18:02:25 +02:00 committed by GitHub
parent d2d2797af9
commit 0c1c33a2a3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 65 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/async_wrap.cc
View File

@ -193,6 +193,12 @@ static void SetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
args[3]->IsFunction() ? args[3].As<Function>() : Local<Function>());
}
static void GetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
args.GetReturnValue().Set(
env->async_hooks()->GetPromiseHooks(args.GetIsolate()));
}
class DestroyParam {
public:
double asyncId;
@ -364,6 +370,7 @@ void AsyncWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
SetMethod(isolate, target, "clearAsyncIdStack", ClearAsyncIdStack);
SetMethod(isolate, target, "queueDestroyAsyncId", QueueDestroyAsyncId);
SetMethod(isolate, target, "setPromiseHooks", SetPromiseHooks);
SetMethod(isolate, target, "getPromiseHooks", GetPromiseHooks);
SetMethod(isolate, target, "registerDestroyHook", RegisterDestroyHook);
AsyncWrap::GetConstructorTemplate(isolate_data);
}
@ -469,6 +476,7 @@ void AsyncWrap::RegisterExternalReferences(
registry->Register(ClearAsyncIdStack);
registry->Register(QueueDestroyAsyncId);
registry->Register(SetPromiseHooks);
registry->Register(GetPromiseHooks);
registry->Register(RegisterDestroyHook);
registry->Register(AsyncWrap::GetAsyncId);
registry->Register(AsyncWrap::AsyncReset);

12
src/env.cc
View File

@ -87,6 +87,18 @@ void AsyncHooks::ResetPromiseHooks(Local<Function> init,
js_promise_hooks_[3].Reset(env()->isolate(), resolve);
}
Local<Array> AsyncHooks::GetPromiseHooks(Isolate* isolate) {
std::vector<Local<Value>> values;
for (size_t i = 0; i < js_promise_hooks_.size(); ++i) {
if (js_promise_hooks_[i].IsEmpty()) {
values.push_back(Undefined(isolate));
} else {
values.push_back(js_promise_hooks_[i].Get(isolate));
}
}
return Array::New(isolate, values.data(), values.size());
}
void Environment::ResetPromiseHooks(Local<Function> init,
Local<Function> before,
Local<Function> after,

4
src/env.h
View File

@ -322,7 +322,9 @@ class AsyncHooks : public MemoryRetainer {
v8::Local<v8::Function> before,
v8::Local<v8::Function> after,
v8::Local<v8::Function> resolve);
// Used for testing since V8 doesn't provide API for retrieving configured
// JS promise hooks.
v8::Local<v8::Array> GetPromiseHooks(v8::Isolate* isolate);
inline v8::Local<v8::String> provider_string(int idx);
inline void no_force_checks();

2
src/inspector_agent.cc
View File

@ -923,7 +923,7 @@ void Agent::EnableAsyncHook() {
void Agent::DisableAsyncHook() {
HandleScope scope(parent_env_->isolate());
Local<Function> disable = parent_env_->inspector_enable_async_hooks();
Local<Function> disable = parent_env_->inspector_disable_async_hooks();
if (!disable.IsEmpty()) {
ToggleAsyncHook(parent_env_->isolate(), disable);
} else if (pending_enable_async_hook_) {

86
test/parallel/test-inspector-async-call-stack.js
View File

@ -5,21 +5,32 @@ common.skipIfInspectorDisabled();
common.skipIf32Bits();
const assert = require('assert');
const { inspect } = require('util');
const { internalBinding } = require('internal/test/binding');
const { async_hook_fields, constants } = internalBinding('async_wrap');
const { async_hook_fields, constants, getPromiseHooks } = internalBinding('async_wrap');
const { kTotals } = constants;
const inspector = require('inspector');
const inspector = require('inspector/promises');
const setDepth = 'Debugger.setAsyncCallStackDepth';
const emptyPromiseHooks = [ undefined, undefined, undefined, undefined ];
function verifyAsyncHookDisabled(message) {
assert.strictEqual(async_hook_fields[kTotals], 0,
`${async_hook_fields[kTotals]} !== 0: ${message}`);
const promiseHooks = getPromiseHooks();
assert.deepStrictEqual(
promiseHooks, emptyPromiseHooks,
`${message}: promise hooks ${inspect(promiseHooks)}`
);
}
function verifyAsyncHookEnabled(message) {
assert.strictEqual(async_hook_fields[kTotals], 4,
`${async_hook_fields[kTotals]} !== 4: ${message}`);
const promiseHooks = getPromiseHooks();
assert.notDeepStrictEqual(
promiseHooks, emptyPromiseHooks,
`${message}: promise hooks ${inspect(promiseHooks)}`
);
}
// By default inspector async hooks should not have been installed.
@ -31,53 +42,38 @@ verifyAsyncHookDisabled('creating a session should not enable async hooks');
session.connect();
verifyAsyncHookDisabled('connecting a session should not enable async hooks');
session.post('Debugger.enable', () => {
(async () => {
await session.post('Debugger.enable');
verifyAsyncHookDisabled('enabling debugger should not enable async hooks');
await assert.rejects(session.post(setDepth, { invalid: 'message' }), { code: 'ERR_INSPECTOR_COMMAND' });
verifyAsyncHookDisabled('invalid message should not enable async hooks');
await assert.rejects(session.post(setDepth, { maxDepth: 'five' }), { code: 'ERR_INSPECTOR_COMMAND' });
verifyAsyncHookDisabled('invalid maxDepth (string) should not enable ' +
'async hooks');
await assert.rejects(session.post(setDepth, { maxDepth: NaN }), { code: 'ERR_INSPECTOR_COMMAND' });
verifyAsyncHookDisabled('invalid maxDepth (NaN) should not enable ' +
'async hooks');
await session.post(setDepth, { maxDepth: 10 });
verifyAsyncHookEnabled('valid message should enable async hooks');
session.post(setDepth, { invalid: 'message' }, () => {
verifyAsyncHookDisabled('invalid message should not enable async hooks');
await session.post(setDepth, { maxDepth: 0 });
verifyAsyncHookDisabled('Setting maxDepth to 0 should disable ' +
'async hooks');
session.post(setDepth, { maxDepth: 'five' }, () => {
verifyAsyncHookDisabled('invalid maxDepth (string) should not enable ' +
'async hooks');
await session.post(setDepth, { maxDepth: 32 });
verifyAsyncHookEnabled('valid message should enable async hooks');
session.post(setDepth, { maxDepth: NaN }, () => {
verifyAsyncHookDisabled('invalid maxDepth (NaN) should not enable ' +
'async hooks');
await session.post('Debugger.disable');
verifyAsyncHookDisabled('Debugger.disable should disable async hooks');
session.post(setDepth, { maxDepth: 10 }, () => {
verifyAsyncHookEnabled('valid message should enable async hooks');
await session.post('Debugger.enable');
verifyAsyncHookDisabled('Enabling debugger should not enable hooks');
session.post(setDepth, { maxDepth: 0 }, () => {
verifyAsyncHookDisabled('Setting maxDepth to 0 should disable ' +
'async hooks');
await session.post(setDepth, { maxDepth: 64 });
verifyAsyncHookEnabled('valid message should enable async hooks');
runTestSet2(session);
});
});
});
});
});
});
await session.disconnect();
function runTestSet2(session) {
session.post(setDepth, { maxDepth: 32 }, () => {
verifyAsyncHookEnabled('valid message should enable async hooks');
session.post('Debugger.disable', () => {
verifyAsyncHookDisabled('Debugger.disable should disable async hooks');
session.post('Debugger.enable', () => {
verifyAsyncHookDisabled('Enabling debugger should not enable hooks');
session.post(setDepth, { maxDepth: 64 }, () => {
verifyAsyncHookEnabled('valid message should enable async hooks');
session.disconnect();
verifyAsyncHookDisabled('Disconnecting session should disable ' +
'async hooks');
});
});
});
});
}
verifyAsyncHookDisabled('Disconnecting session should disable ' +
'async hooks');
})().then(common.mustCall());