FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth · nodejs/node@ad5282e · GitHub

/ node Public

Commit ad5282e

Browse files
joyeecheung
authored and
targos
committed
inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth
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: #53473 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent d2c169a commit ad5282e

5 files changed

Lines changed: 72 additions & 54 deletions

File tree

‎src/async_wrap.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,12 @@ static void SetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
193193
args[3]->IsFunction() ? args[3].As<Function>() : Local<Function>());
194194
}
195195

196+
static void GetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
197+
Environment* env = Environment::GetCurrent(args);
198+
args.GetReturnValue().Set(
199+
env->async_hooks()->GetPromiseHooks(args.GetIsolate()));
200+
}
201+
196202
class DestroyParam {
197203
public:
198204
double asyncId;
@@ -364,6 +370,7 @@ void AsyncWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
364370
SetMethod(isolate, target, "clearAsyncIdStack", ClearAsyncIdStack);
365371
SetMethod(isolate, target, "queueDestroyAsyncId", QueueDestroyAsyncId);
366372
SetMethod(isolate, target, "setPromiseHooks", SetPromiseHooks);
373+
SetMethod(isolate, target, "getPromiseHooks", GetPromiseHooks);
367374
SetMethod(isolate, target, "registerDestroyHook", RegisterDestroyHook);
368375
AsyncWrap::GetConstructorTemplate(isolate_data);
369376
}
@@ -469,6 +476,7 @@ void AsyncWrap::RegisterExternalReferences(
469476
registry->Register(ClearAsyncIdStack);
470477
registry->Register(QueueDestroyAsyncId);
471478
registry->Register(SetPromiseHooks);
479+
registry->Register(GetPromiseHooks);
472480
registry->Register(RegisterDestroyHook);
473481
registry->Register(AsyncWrap::GetAsyncId);
474482
registry->Register(AsyncWrap::AsyncReset);

‎src/env.cc‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,18 @@ void AsyncHooks::ResetPromiseHooks(Local<Function> init,
8787
js_promise_hooks_[3].Reset(env()->isolate(), resolve);
8888
}
8989

90+
Local<Array> AsyncHooks::GetPromiseHooks(Isolate* isolate) {
91+
std::vector<Local<Value>> values;
92+
for (size_t i = 0; i < js_promise_hooks_.size(); ++i) {
93+
if (js_promise_hooks_[i].IsEmpty()) {
94+
values.push_back(Undefined(isolate));
95+
} else {
96+
values.push_back(js_promise_hooks_[i].Get(isolate));
97+
}
98+
}
99+
return Array::New(isolate, values.data(), values.size());
100+
}
101+
90102
void Environment::ResetPromiseHooks(Local<Function> init,
91103
Local<Function> before,
92104
Local<Function> after,

‎src/env.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,9 @@ class AsyncHooks : public MemoryRetainer {
322322
v8::Local<v8::Function> before,
323323
v8::Local<v8::Function> after,
324324
v8::Local<v8::Function> resolve);
325-
325+
// Used for testing since V8 doesn't provide API for retrieving configured
326+
// JS promise hooks.
327+
v8::Local<v8::Array> GetPromiseHooks(v8::Isolate* isolate);
326328
inline v8::Local<v8::String> provider_string(int idx);
327329

328330
inline void no_force_checks();

‎src/inspector_agent.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -923,7 +923,7 @@ void Agent::EnableAsyncHook() {
923923

924924
void Agent::DisableAsyncHook() {
925925
HandleScope scope(parent_env_->isolate());
926-
Local<Function> disable = parent_env_->inspector_enable_async_hooks();
926+
Local<Function> disable = parent_env_->inspector_disable_async_hooks();
927927
if (!disable.IsEmpty()) {
928928
ToggleAsyncHook(parent_env_->isolate(), disable);
929929
} else if (pending_enable_async_hook_) {

‎test/parallel/test-inspector-async-call-stack.js‎

Lines changed: 48 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,32 @@ common.skipIfInspectorDisabled();
55
common.skipIf32Bits();
66

77
const assert = require('assert');
8+
const { inspect } = require('util');
89
const { internalBinding } = require('internal/test/binding');
9-
const { async_hook_fields, constants } = internalBinding('async_wrap');
10+
const { async_hook_fields, constants, getPromiseHooks } = internalBinding('async_wrap');
1011
const { kTotals } = constants;
11-
const inspector = require('inspector');
12+
const inspector = require('inspector/promises');
1213

1314
const setDepth = 'Debugger.setAsyncCallStackDepth';
14-
15+
const emptyPromiseHooks = [ undefined, undefined, undefined, undefined ];
1516
function verifyAsyncHookDisabled(message) {
1617
assert.strictEqual(async_hook_fields[kTotals], 0,
1718
`${async_hook_fields[kTotals]} !== 0: ${message}`);
19+
const promiseHooks = getPromiseHooks();
20+
assert.deepStrictEqual(
21+
promiseHooks, emptyPromiseHooks,
22+
`${message}: promise hooks ${inspect(promiseHooks)}`
23+
);
1824
}
1925

2026
function verifyAsyncHookEnabled(message) {
2127
assert.strictEqual(async_hook_fields[kTotals], 4,
2228
`${async_hook_fields[kTotals]} !== 4: ${message}`);
29+
const promiseHooks = getPromiseHooks();
30+
assert.notDeepStrictEqual(
31+
promiseHooks, emptyPromiseHooks,
32+
`${message}: promise hooks ${inspect(promiseHooks)}`
33+
);
2334
}
2435

2536
// By default inspector async hooks should not have been installed.
@@ -31,53 +42,38 @@ verifyAsyncHookDisabled('creating a session should not enable async hooks');
3142
session.connect();
3243
verifyAsyncHookDisabled('connecting a session should not enable async hooks');
3344

34-
session.post('Debugger.enable', () => {
45+
(async () => {
46+
await session.post('Debugger.enable');
3547
verifyAsyncHookDisabled('enabling debugger should not enable async hooks');
36-
37-
session.post(setDepth, { invalid: 'message' }, () => {
38-
verifyAsyncHookDisabled('invalid message should not enable async hooks');
39-
40-
session.post(setDepth, { maxDepth: 'five' }, () => {
41-
verifyAsyncHookDisabled('invalid maxDepth (string) should not enable ' +
42-
'async hooks');
43-
44-
session.post(setDepth, { maxDepth: NaN }, () => {
45-
verifyAsyncHookDisabled('invalid maxDepth (NaN) should not enable ' +
46-
'async hooks');
47-
48-
session.post(setDepth, { maxDepth: 10 }, () => {
49-
verifyAsyncHookEnabled('valid message should enable async hooks');
50-
51-
session.post(setDepth, { maxDepth: 0 }, () => {
52-
verifyAsyncHookDisabled('Setting maxDepth to 0 should disable ' +
53-
'async hooks');
54-
55-
runTestSet2(session);
56-
});
57-
});
58-
});
59-
});
60-
});
61-
});
62-
63-
function runTestSet2(session) {
64-
session.post(setDepth, { maxDepth: 32 }, () => {
65-
verifyAsyncHookEnabled('valid message should enable async hooks');
66-
67-
session.post('Debugger.disable', () => {
68-
verifyAsyncHookDisabled('Debugger.disable should disable async hooks');
69-
70-
session.post('Debugger.enable', () => {
71-
verifyAsyncHookDisabled('Enabling debugger should not enable hooks');
72-
73-
session.post(setDepth, { maxDepth: 64 }, () => {
74-
verifyAsyncHookEnabled('valid message should enable async hooks');
75-
76-
session.disconnect();
77-
verifyAsyncHookDisabled('Disconnecting session should disable ' +
78-
'async hooks');
79-
});
80-
});
81-
});
82-
});
83-
}
48+
await assert.rejects(session.post(setDepth, { invalid: 'message' }), { code: 'ERR_INSPECTOR_COMMAND' });
49+
verifyAsyncHookDisabled('invalid message should not enable async hooks');
50+
await assert.rejects(session.post(setDepth, { maxDepth: 'five' }), { code: 'ERR_INSPECTOR_COMMAND' });
51+
verifyAsyncHookDisabled('invalid maxDepth (string) should not enable ' +
52+
'async hooks');
53+
await assert.rejects(session.post(setDepth, { maxDepth: NaN }), { code: 'ERR_INSPECTOR_COMMAND' });
54+
verifyAsyncHookDisabled('invalid maxDepth (NaN) should not enable ' +
55+
'async hooks');
56+
await session.post(setDepth, { maxDepth: 10 });
57+
verifyAsyncHookEnabled('valid message should enable async hooks');
58+
59+
await session.post(setDepth, { maxDepth: 0 });
60+
verifyAsyncHookDisabled('Setting maxDepth to 0 should disable ' +
61+
'async hooks');
62+
63+
await session.post(setDepth, { maxDepth: 32 });
64+
verifyAsyncHookEnabled('valid message should enable async hooks');
65+
66+
await session.post('Debugger.disable');
67+
verifyAsyncHookDisabled('Debugger.disable should disable async hooks');
68+
69+
await session.post('Debugger.enable');
70+
verifyAsyncHookDisabled('Enabling debugger should not enable hooks');
71+
72+
await session.post(setDepth, { maxDepth: 64 });
73+
verifyAsyncHookEnabled('valid message should enable async hooks');
74+
75+
await session.disconnect();
76+
77+
verifyAsyncHookDisabled('Disconnecting session should disable ' +
78+
'async hooks');
79+
})().then(common.mustCall());

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL