GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth by joyeecheung · Pull Request #53473 · nodejs/node

Conversation

joyeecheung commented Jun 16, 2024

Copy link

Copy Markdown

Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

fabiospampinato, bricss, tonivj5, MomenNano, H4ad, ovflowd, sheremet-va, rschristian, and paulirish reacted with heart emoji

All reactions

❤️ 9 reactions


          inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth

601ad09

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.

nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

inspector

Issues and PRs related to the V8 inspector protocol.

needs-ci PRs that need a full CI run. labels

Jun 16, 2024

targos approved these changes

Jun 16, 2024

View reviewed changes


          fixup! inspector: fix disable async hooks on Debugger.setAsyncCallSta…

db19635

…ckDepth

joyeecheung commented Jun 16, 2024

Copy link

Copy Markdown

Member Author

Apparently there is a test in test/parallel/test-inspector-async-call-stack.js but it was not working as intended because errors thrown in the session callbacks are not handled by the process. That seems to be a different bug (or is that working as intended? I guess not? cc @nodejs/inspector ). As a result, the test was not failing even though the assertions failed.For this PR, I will just rewrite the test to use inspector/promises which allows proper error handling.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          fixup! fixup! inspector: fix disable async hooks on Debugger.setAsync…

…CallStackDepth

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Jun 16, 2024

joyeecheung commented Jun 16, 2024

Copy link

Copy Markdown

Member Author

Added another test for checking promise hooks - it needs a utility to check the JS hooks stored in the AsyncHooks since V8 doesn't provide an API for us to check the configured JS promise hooks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Jun 16, 2024

nodejs-github-bot commented Jun 16, 2024

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59819/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anonrig approved these changes

Jun 16, 2024

View reviewed changes

benjamingr approved these changes

Jun 16, 2024

View reviewed changes

joyeecheung added notable-change

PRs with changes that should be highlighted in changelogs.

author ready PRs with CI started, the required approvals, and no outstanding review comments. labels

Jun 17, 2024

github-actions Bot commented Jun 17, 2024

Copy link

Copy Markdown

Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @joyeecheung.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anonrig added the commit-queue PRs queued for automated landing through the Commit Queue. label

Jun 17, 2024

joyeecheung added the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label

Jun 18, 2024

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Jun 18, 2024

nodejs-github-bot merged commit 0c1c33a into nodejs:main

Jun 18, 2024

nodejs-github-bot commented Jun 18, 2024

Copy link

Copy Markdown

Collaborator

Landed in 0c1c33a

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos pushed a commit that referenced this pull request

Jun 20, 2024


          inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth

ad5282e

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 <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>

soophoo pushed a commit to soophoo/node that referenced this pull request

Jun 20, 2024


          inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth

a37611c

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: nodejs#53473
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>

targos added a commit that referenced this pull request

Jun 25, 2024


          2024-06-26, Version 22.4.0 (Current)

8efb279

Notable changes:

deps,lib,src:
  * (SEMVER-MINOR) add experimental web storage (Colin Ihrig) #52435
doc:
  * move `node --run` stability to rc (Yagiz Nizipli) #53433
  * mark WebSocket as stable (Matthew Aitken) #53352
  * mark --heap-prof and related flags stable (Joyee Cheung) #53343
  * mark --cpu-prof and related flags stable (Joyee Cheung) #53343
  * doc-only deprecate OpenSSL engine-based APIs (Richard Lau) #53329
inspector:
  * fix disable async hooks on Debugger.setAsyncCallStackDepth (Joyee Cheung) #53473
lib:
  * (SEMVER-MINOR) add diagnostics_channel events to module loading (RafaelGSS) #44340
util:
  * (SEMVER-MINOR) support `--no-` for argument with boolean type for parseArgs (Zhenwei Jin) #53107

PR-URL: #53583

targos mentioned this pull request

Jun 25, 2024

v22.4.0 release proposal #53583

Merged

targos added a commit that referenced this pull request

Jun 26, 2024


          2024-06-26, Version 22.4.0 (Current)

e7e7e93

Notable changes:

deps,lib,src:
  * (SEMVER-MINOR) add experimental web storage (Colin Ihrig) #52435
doc:
  * move `node --run` stability to rc (Yagiz Nizipli) #53433
  * mark WebSocket as stable (Matthew Aitken) #53352
  * mark --heap-prof and related flags stable (Joyee Cheung) #53343
  * mark --cpu-prof and related flags stable (Joyee Cheung) #53343
  * doc-only deprecate OpenSSL engine-based APIs (Richard Lau) #53329
inspector:
  * fix disable async hooks on Debugger.setAsyncCallStackDepth (Joyee Cheung) #53473
lib:
  * (SEMVER-MINOR) add diagnostics_channel events to module loading (RafaelGSS) #44340
util:
  * (SEMVER-MINOR) support `--no-` for argument with boolean type for parseArgs (Zhenwei Jin) #53107

PR-URL: #53583

targos added a commit that referenced this pull request

Jul 1, 2024


          2024-07-02, Version 22.4.0 (Current)

7e13cc5

Notable changes:

deps,lib,src:
  * (SEMVER-MINOR) add experimental web storage (Colin Ihrig) #52435
doc:
  * move `node --run` stability to rc (Yagiz Nizipli) #53433
  * mark WebSocket as stable (Matthew Aitken) #53352
  * mark --heap-prof and related flags stable (Joyee Cheung) #53343
  * mark --cpu-prof and related flags stable (Joyee Cheung) #53343
  * doc-only deprecate OpenSSL engine-based APIs (Richard Lau) #53329
inspector:
  * fix disable async hooks on Debugger.setAsyncCallStackDepth (Joyee Cheung) #53473
lib:
  * (SEMVER-MINOR) add diagnostics_channel events to module loading (RafaelGSS) #44340
util:
  * (SEMVER-MINOR) support `--no-` for argument with boolean type for parseArgs (Zhenwei Jin) #53107

PR-URL: #53583

targos added a commit that referenced this pull request

Jul 2, 2024


          2024-07-02, Version 22.4.0 (Current)

056f055

Notable changes:

deps,lib,src:
  * (SEMVER-MINOR) add experimental web storage (Colin Ihrig) #52435
doc:
  * move `node --run` stability to rc (Yagiz Nizipli) #53433
  * mark WebSocket as stable (Matthew Aitken) #53352
  * mark --heap-prof and related flags stable (Joyee Cheung) #53343
  * mark --cpu-prof and related flags stable (Joyee Cheung) #53343
  * doc-only deprecate OpenSSL engine-based APIs (Richard Lau) #53329
inspector:
  * fix disable async hooks on Debugger.setAsyncCallStackDepth (Joyee Cheung) #53473
lib:
  * (SEMVER-MINOR) add diagnostics_channel events to module loading (RafaelGSS) #44340
util:
  * (SEMVER-MINOR) support `--no-` for argument with boolean type for parseArgs (Zhenwei Jin) #53107

PR-URL: #53583

Karliz24 mentioned this pull request

Jul 9, 2024

## 2024-07-02, Version 22.4.0 (Current), @targos Karliz24/Marley-#16

Open

marco-ippolito pushed a commit that referenced this pull request

Jul 19, 2024


          inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth

eb0b68b

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 <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>

marco-ippolito mentioned this pull request

Jul 19, 2024

v20.16.0 proposal #53945

Merged

marco-ippolito added a commit that referenced this pull request

Jul 19, 2024


          2024-07-24, Version 20.16.0 'Iron' (LTS)

7d0c53d

Notable changes:

buffer:
  * (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
doc:
  * doc-only deprecate OpenSSL engine-based APIs (Richard Lau) #53329
  * (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
  * (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
  * add StefanStojanovic to collaborators (StefanStojanovic) #53118
  * add Marco Ippolito to TSC (Rafael Gonzaga) #53008
inspector:
  * fix disable async hooks on Debugger.setAsyncCallStackDepth (Joyee Cheung) #53473
net:
  * (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
  * (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src,permission:
  * (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
  * (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
util:
  * (SEMVER-MINOR) support `--no-` for argument with boolean type for parseArgs (Zhenwei Jin) #53107

PR-URL: #53945

marco-ippolito added a commit that referenced this pull request

Jul 21, 2024


          2024-07-24, Version 20.16.0 'Iron' (LTS)

1968ef3

Notable changes:

buffer:
  * (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
doc:
  * doc-only deprecate OpenSSL engine-based APIs (Richard Lau) #53329
  * (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
  * (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
  * add StefanStojanovic to collaborators (StefanStojanovic) #53118
  * add Marco Ippolito to TSC (Rafael Gonzaga) #53008
inspector:
  * fix disable async hooks on Debugger.setAsyncCallStackDepth (Joyee Cheung) #53473
net:
  * (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
  * (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src,permission:
  * (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
  * (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
util:
  * (SEMVER-MINOR) support `--no-` for argument with boolean type for parseArgs (Zhenwei Jin) #53107

PR-URL: #53945

marco-ippolito added a commit that referenced this pull request

Jul 24, 2024


          2024-07-24, Version 20.16.0 'Iron' (LTS)

7a6185d

Notable changes:

buffer:
  * (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
doc:
  * doc-only deprecate OpenSSL engine-based APIs (Richard Lau) #53329
  * (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
  * (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
  * add StefanStojanovic to collaborators (StefanStojanovic) #53118
  * add Marco Ippolito to TSC (Rafael Gonzaga) #53008
inspector:
  * fix disable async hooks on Debugger.setAsyncCallStackDepth (Joyee Cheung) #53473
net:
  * (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
  * (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src,permission:
  * (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
  * (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
util:
  * (SEMVER-MINOR) support `--no-` for argument with boolean type for parseArgs (Zhenwei Jin) #53107

PR-URL: #53945

Reviewers

benjamingr

benjamingr approved these changes

anonrig

anonrig approved these changes

+1 more reviewer

targos

targos approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

c++

Issues and PRs that require attention from people who are familiar with C++.

commit-queue-squash

PRs the Commit Queue should land as one squashed commit.

inspector

Issues and PRs related to the V8 inspector protocol.

needs-ci

PRs that need a full CI run.

notable-change

PRs with changes that should be highlighted in changelogs.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.