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

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
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
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
@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
`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
MylesBorins pushed a commit that referenced this issue Dec 12, 2017
`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 <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
gibfahn pushed a commit that referenced this issue Dec 19, 2017
`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 <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
gibfahn pushed a commit that referenced this issue Dec 20, 2017
`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 <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants