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 failed in V8 for parallel/test-error-reporting #17016

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

Debug build assertion failed in V8 for parallel/test-error-reporting #17016

rvagg opened this issue Nov 14, 2017 · 8 comments
Assignees
Labels
test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency.

Comments

@rvagg
Copy link
Member

rvagg commented Nov 14, 2017

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 parallel/test-error-reporting:

$ ./out/Debug/node ./test/fixtures/throws_error7.js


#
# Fatal error in ../deps/v8/src/api.cc, line 230
# Debug check failed: !isolate_->external_caught_exception().
#
Illegal instruction: 4

/cc @nodejs/v8

@rvagg
Copy link
Member Author

rvagg commented Nov 14, 2017

Fails on Linux and macOS

@bnoordhuis
Copy link
Member

Stack trace:

#0  v8::base::OS::Abort () at ../deps/v8/src/base/platform/platform-posix.cc:248
#1  0x00005555577cc5f9 in V8_Fatal (file=0x55555792f450 "../deps/v8/src/api.cc", line=230, format=0x55555792bd4a "Debug check failed: %s.") at ../deps/v8/src/base/logging.cc:126
#2  0x00005555568ec97b in v8::(anonymous namespace)::CallDepthScope<false>::CallDepthScope (this=0x7fffffffc2d0, isolate=0x555558b56320, context=...) at ../deps/v8/src/api.cc:230
#3  0x00005555568c2cd1 in v8::Object::Get (this=0x555558bbf760, context=..., key=...) at ../deps/v8/src/api.cc:4525
#4  0x00005555568c2f29 in v8::Object::Get (this=0x555558bbf760, key=...) at ../deps/v8/src/api.cc:4538
#5  0x0000555557528e69 in node::FatalException (isolate=0x555558b56320, error=..., message=...) at ../src/node.cc:2490
#6  0x0000555557529119 in node::OnMessage (message=..., error=...) at ../src/node.cc:2542
#7  0x00005555570ac1ae in v8::internal::MessageHandler::ReportMessageNoExceptions (isolate=0x555558b56320, loc=0x7fffffffc6f0, message=..., api_exception_obj=...) at ../deps/v8/src/messages.cc:161
#8  0x00005555570abe93 in v8::internal::MessageHandler::ReportMessage (isolate=0x555558b56320, loc=0x7fffffffc6f0, message=...) at ../deps/v8/src/messages.cc:124
#9  0x000055555706f9c0 in v8::internal::Isolate::ReportPendingMessages (this=0x555558b56320) at ../deps/v8/src/isolate.cc:1792
#10 0x0000555556ea7acc in v8::internal::(anonymous namespace)::Invoke (isolate=0x555558b56320, is_construct=false, target=..., receiver=..., argc=1, args=0x7fffffffca78, new_target=..., message_handling=v8::internal::Execution::MessageHandling::kReport) at ../deps/v8/src/execution.cc:160
#11 0x0000555556ea7c36 in v8::internal::(anonymous namespace)::CallInternal (isolate=0x555558b56320, callable=..., receiver=..., argc=1, argv=0x7fffffffca78, message_handling=v8::internal::Execution::MessageHandling::kReport) at ../deps/v8/src/execution.cc:182
#12 0x0000555556ea7c8f in v8::internal::Execution::Call (isolate=0x555558b56320, callable=..., receiver=..., argc=1, argv=0x7fffffffca78) at ../deps/v8/src/execution.cc:192
#13 0x00005555568c8f40 in v8::Function::Call (this=0x555558bbbb48, context=..., recv=..., argc=1, argv=0x7fffffffca78) at ../deps/v8/src/api.cc:5311
#14 0x000055555752e769 in node::LoadEnvironment (env=0x7fffffffcbd0) at ../src/node.cc:3562
#15 0x000055555753462b in node::Start (isolate=0x555558b56320, isolate_data=0x7fffffffd380, argc=2, argv=0x555558b55240, exec_argc=0, exec_argv=0x555558b54f90) at ../src/node.cc:4550
#16 0x0000555557534a8a in node::Start (event_loop=0x555558b3dde0 <default_loop_struct>, argc=2, argv=0x555558b55240, exec_argc=0, exec_argv=0x555558b54f90) at ../src/node.cc:4633
#17 0x0000555557530b5c in node::Start (argc=2, argv=0x555558b55240) at ../src/node.cc:4690
#18 0x00005555575a0797 in main (argc=2, argv=0x7fffffffdc08) at ../src/node_main.cc:106

@hashseed
Copy link
Member

hashseed commented Nov 14, 2017

Issue here is that there is still a pending exception on the Isolate when you call into V8 again through Object::Get.

The message callback is really just there to print messages to the console or stdout/stderr, not to execute JS. At this point the exception has not been handled yet, and if the new JS throws, that exception would be overwritten. This callback should be done when frame #13 returns with an empty handle.

@mscdex mscdex added test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency. labels Nov 14, 2017
@bnoordhuis
Copy link
Member

This callback should be done when frame #13 returns with an empty handle.

FTR, I figured this out and am putting together a pull request. =)

@addaleax
Copy link
Member

Issue here is that there is still a pending exception on the Isolate when you call into V8 again through Object::Get.

Are you sure that that’s true? By the time the message listener callback is executed, all the stack has unwound.

Also, and more importantly, that is not what the V8 API seems to support; quoting the commit message from my initial (failing) attempt to fix this in V8 (https://chromium-review.googlesource.com/c/v8/v8/+/718096):

The existence of an AllowJavascriptExecutionDebugOnly scope in
Isolate::ReportPendingMessages() indicates that the API supports
running arbitrary JS code in a AddMessageListener callback.

This is also specific to the case in which toString() for the exception fails when V8 tries to invoke that method; not for general exceptions being thrown inside the message handler. So I am pretty sure this is a bug in V8.

@addaleax
Copy link
Member

Okay, I think I really understand what’s going on now. The fix in https://chromium-review.googlesource.com/c/v8/v8/+/718096 should work for this.

@addaleax
Copy link
Member

addaleax commented Dec 8, 2017

This can be closed now that V8 6.3 has landed.

Yay!

@addaleax addaleax closed this as completed Dec 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

5 participants