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.

Sign up for GitHub

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

Jump to bottom

Debug build assertion failure in V8 for sequential/test-inspector-async-stack-traces-promise-then #17017

Closed
rvagg opened this issue Nov 14, 2017 · 3 comments
Closed

Debug build assertion failure in V8 for sequential/test-inspector-async-stack-traces-promise-then #17017

rvagg opened this issue Nov 14, 2017 · 3 comments
Labels
inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests.

Comments

@rvagg
Copy link
Member

rvagg commented Nov 14, 2017

(as per #17016)

Testing out adding Debug builds to CI and I have some failures that need looking at that'll need someone cleverer than me.

Current master, fails via sequential/test-inspector-async-stack-traces-promise-then:

$ ./out/Debug/node test/sequential/test-inspector-async-stack-traces-promise-then
[test] Connecting to a child Node process
[test] Testing /json/list
[err] Debugger listening on ws://127.0.0.1:58956/bdb5e1e8-9a06-4d52-9d2b-ba331c6de620
[err] For help see https://nodejs.org/en/docs/inspector
[err]
[err] Debugger attached.
[err]
[test] Waiting for break1
[test] Waiting for break2
[err]
[err]
[err] #
[err] # Fatal error in ../deps/v8/src/inspector/v8-debugger.cc, line 831
[err] # Debug check failed: m_currentTasks.back() == task.
[err] #
[err]
Timed out waiting for matching notification (break on [eval]:9))
1

tfw debug build is failing in its debugger

/cc @nodejs/v8
@rvagg rvagg mentioned this issue Nov 14, 2017
Closed
@rvagg
Copy link
Member Author

rvagg commented Nov 14, 2017

Fails on Linux and macOS

@hashseed
Copy link
Member

@ak239

@mscdex mscdex added inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests. labels Nov 14, 2017
@rvagg rvagg mentioned this issue Nov 14, 2017
Closed
@addaleax
Copy link
Member

See https://chromium-review.googlesource.com/c/v8/v8/+/707058 and the discussion in it – essentially, we need to turn off the reporting of our own promise tracking to V8, or teach V8 to not track promises itself, or apply my 10-line fix to make sure the tracking ordering is correct.

addaleax added a commit to addaleax/node that referenced this issue Nov 18, 2017
@addaleax
inspector: no async tracking for promises
0fb6f70 
`Promise` instances are already tracked by V8 itself.
This fixes `sequential/test-inspector-async-stack-traces-promise-then`
in debug mode (it previously crashed because our tracking
and the V8 tracking were not properly nested).

Ref: https://chromium-review.googlesource.com/c/v8/v8/+/707058
Fixes: nodejs#17017
@addaleax addaleax mentioned this issue Nov 18, 2017
Closed
3 tasks
MylesBorins pushed a commit that referenced this issue Dec 12, 2017
@addaleax @MylesBorins
inspector: no async tracking for promises
94e0488 
`Promise` instances are already tracked by V8 itself.
This fixes `sequential/test-inspector-async-stack-traces-promise-then`
in debug mode (it previously crashed because our tracking
and the V8 tracking were not properly nested).

PR-URL: #17118
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/707058
Fixes: #17017
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this issue Dec 19, 2017
@addaleax @gibfahn
inspector: no async tracking for promises
33d04fd 
`Promise` instances are already tracked by V8 itself.
This fixes `sequential/test-inspector-async-stack-traces-promise-then`
in debug mode (it previously crashed because our tracking
and the V8 tracking were not properly nested).

PR-URL: #17118
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/707058
Fixes: #17017
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this issue Dec 20, 2017
@addaleax @gibfahn
inspector: no async tracking for promises
4b43e52 
`Promise` instances are already tracked by V8 itself.
This fixes `sequential/test-inspector-async-stack-traces-promise-then`
in debug mode (it previously crashed because our tracking
and the V8 tracking were not properly nested).

PR-URL: #17118
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/707058
Fixes: #17017
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

inspector: no async tracking for promises addaleax/node
4 participants
@mscdex @rvagg @addaleax @hashseed