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

[rvagg]

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]

Fails on Linux and macOS

[bnoordhuis]

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]

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.

[bnoordhuis]

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]

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]

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]

Upstream merge request for the fix: https://bugs.chromium.org/p/v8/issues/detail?id=7139&q=&sort=-id&colspec=ID%20Type%20Status%20Priority%20Owner%20Summary%20HW%20OS%20Component%20Stars

[addaleax]

This can be closed now that V8 6.3 has landed.

Yay!