Skip to content
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

inspector: not pass zero as task_id #15740

Closed
wants to merge 1 commit into from
Closed

inspector: not pass zero as task_id #15740

wants to merge 1 commit into from

Conversation

alexkozy
Copy link
Member

@alexkozy alexkozy commented Oct 2, 2017

Inspector doesn't support zero as task_id. Node.js uses 0 at least
as microTasksTickObject async_id in next_tick.js.
We can ignore this task and not report it to asyncTask* API.

Fixes: #15464

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

inspector

Inspector doesn't support zero as task_id. Node.js uses 0 at least
as microTasksTickObject async_id in next_tick.js.
We can ignore this task and not report it to asyncTask* API.

Fixes: #15464
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x inspector Issues and PRs related to the V8 inspector protocol labels Oct 2, 2017
@alexkozy
Copy link
Member Author

alexkozy commented Oct 2, 2017

I've CL for inspector inside V8 as well: https://chromium-review.googlesource.com/c/v8/v8/+/695808
We can either merge this one or merge V8 change as soon as it is landed.

@alexkozy
Copy link
Member Author

alexkozy commented Oct 2, 2017

I'm actually not sure about good fix here and need help from someone experienced in async_hooks. I'm not sure what zero means. If it's not important to include this task into async stack in DevTools then we can just ignore it, if it's important we can report async_task_id + 1 to inspector.

@MylesBorins
Copy link
Contributor

Closing in favor of #16053

Thanks for putting this together @ak239

@MylesBorins MylesBorins closed this Oct 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debugging v8.5 - always pauses in async_hooks.js on promise rejection
3 participants