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

test-inspector-vm-global-accessors-getter-sideeffect fails with debug build #274

Closed
targos opened this issue Dec 7, 2023 · 6 comments
Closed

Comments

@targos
Copy link
Member

targos commented Dec 7, 2023

$ out/Debug/node test/parallel/test-inspector-vm-global-accessors-getter-sideeffect.js


#
# Fatal error in ../../deps/v8/src/execution/isolate.cc, line 1839
# Debug check failed: !has_exception().
#
#
#
#FailureMessage Object: 0x16d204cb8
----- Native stack trace -----

 1: 0x102d2226c node::DumpNativeBacktrace(__sFILE*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
 2: 0x102f66e6c node::NodePlatform::GetStackTracePrinter()::$_3::operator()() const [/Users/mzasso/git/nodejs/canary/out/Debug/node]
 3: 0x102f66e28 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [/Users/mzasso/git/nodejs/canary/out/Debug/node]
 4: 0x105055c80 V8_Fatal(char const*, int, char const*, ...) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
 5: 0x10505581c std::__1::enable_if<!std::is_function<std::__1::remove_pointer<char>::type>::value && !std::is_enum<char>::value && has_output_operator<char, v8::base::CheckMessageStream>::value, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>::type v8::base::PrintCheckOperand<char>(char) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
 6: 0x10358253c v8::internal::Isolate::Throw(v8::internal::Tagged<v8::internal::Object>, v8::internal::MessageLocation*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
 7: 0x1034ded24 v8::internal::Debug::StopSideEffectCheckMode() [/Users/mzasso/git/nodejs/canary/out/Debug/node]
 8: 0x1034ae684 v8::internal::DebugEvaluate::Global(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSFunction>, v8::debug::EvaluateGlobalMode, v8::internal::REPLMode) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
 9: 0x1034ae418 v8::internal::DebugEvaluate::Global(v8::internal::Isolate*, v8::internal::Handle<v8::internal::String>, v8::debug::EvaluateGlobalMode, v8::internal::REPLMode) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
10: 0x1034bd608 v8::debug::EvaluateGlobal(v8::Isolate*, v8::Local<v8::String>, v8::debug::EvaluateGlobalMode, bool) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
11: 0x103f7563c v8_inspector::V8RuntimeAgentImpl::evaluate(v8_inspector::String16 const&, v8_crdtp::detail::ValueMaybe<v8_inspector::String16>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<int>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<double>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<bool>, v8_crdtp::detail::ValueMaybe<v8_inspector::String16>, v8_crdtp::detail::PtrMaybe<v8_inspector::protocol::Runtime::SerializationOptions>, std::__1::unique_ptr<v8_inspector::protocol::Runtime::Backend::EvaluateCallback, std::__1::default_delete<v8_inspector::protocol::Runtime::Backend::EvaluateCallback>>) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
12: 0x1031e9ce4 v8_inspector::protocol::Runtime::DomainDispatcherImpl::evaluate(v8_crdtp::Dispatchable const&) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
13: 0x103f98190 v8_crdtp::UberDispatcher::DispatchResult::Run() [/Users/mzasso/git/nodejs/canary/out/Debug/node]
14: 0x103f6fb68 v8_inspector::V8InspectorSessionImpl::dispatchProtocolMessage(v8_inspector::StringView) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
15: 0x10308a164 node::inspector::(anonymous namespace)::ChannelImpl::dispatchProtocolMessage(v8_inspector::StringView const&) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
16: 0x103089f54 node::inspector::NodeInspectorClient::dispatchMessageFromFrontend(int, v8_inspector::StringView const&) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
17: 0x10308928c node::inspector::(anonymous namespace)::SameThreadInspectorSession::Dispatch(v8_inspector::StringView const&) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
18: 0x1030a88e0 node::inspector::(anonymous namespace)::JSBindingsConnection<node::inspector::(anonymous namespace)::LocalConnection>::Dispatch(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
19: 0x1044e63f8 Builtins_CallApiCallbackGeneric [/Users/mzasso/git/nodejs/canary/out/Debug/node]
20: 0x1044e4370 Builtins_InterpreterEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
21: 0x1044e4370 Builtins_InterpreterEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
22: 0x1044e4370 Builtins_InterpreterEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
23: 0x1044e4370 Builtins_InterpreterEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
24: 0x1044e4370 Builtins_InterpreterEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
25: 0x1044e4370 Builtins_InterpreterEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
26: 0x1044e4370 Builtins_InterpreterEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
27: 0x1044e4370 Builtins_InterpreterEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
28: 0x1044e180c Builtins_JSEntryTrampoline [/Users/mzasso/git/nodejs/canary/out/Debug/node]
29: 0x1044e14f4 Builtins_JSEntry [/Users/mzasso/git/nodejs/canary/out/Debug/node]
30: 0x10355862c v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
31: 0x103557ad4 v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
32: 0x103235f24 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
33: 0x102e1b08c node::builtins::BuiltinLoader::CompileAndCall(v8::Local<v8::Context>, char const*, int, v8::Local<v8::Value>*, node::Realm*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
34: 0x102e1af10 node::builtins::BuiltinLoader::CompileAndCall(v8::Local<v8::Context>, char const*, node::Realm*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
35: 0x102f8a450 node::Realm::ExecuteBootstrapper(char const*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
36: 0x102dd522c node::StartExecution(node::Environment*, char const*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
37: 0x102dd5028 node::StartExecution(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
38: 0x102c75730 node::LoadEnvironment(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
39: 0x102ee59f4 node::NodeMainInstance::Run(node::ExitCode*, node::Environment*) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
40: 0x102ee562c node::NodeMainInstance::Run() [/Users/mzasso/git/nodejs/canary/out/Debug/node]
41: 0x102dd7a80 node::StartInternal(int, char**) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
42: 0x102dd76c0 node::Start(int, char**) [/Users/mzasso/git/nodejs/canary/out/Debug/node]
43: 0x104ac7018 main [/Users/mzasso/git/nodejs/canary/out/Debug/node]
44: 0x1844d10e0 start [/usr/lib/dyld]
[1]    82437 trace trap  out/Debug/node
@targos
Copy link
Member Author

targos commented Dec 22, 2023

https://ci.nodejs.org/job/node-test-commit-arm-debug/11012/nodes=ubuntu2004_debug-arm64/console

12:21:26 not ok 2056 parallel/test-inspector-vm-global-accessors-getter-sideeffect
12:21:26   ---
12:21:26   duration_ms: 619.33800
12:21:26   severity: crashed
12:21:26   exitcode: -5
12:21:26   stack: |-
12:21:26     
12:21:26     
12:21:26     #
12:21:26     # Fatal error in ../deps/v8/src/api/api.cc, line 5242
12:21:26     # Debug check failed: !i_isolate->is_execution_terminating().
12:21:26     #
12:21:26     #
12:21:26     #
12:21:26     #FailureMessage Object: 0xffffc5a5ae08
12:21:26     ----- Native stack trace -----
12:21:26     
12:21:26      1: 0xaaaaebb3e5c0 node::DumpNativeBacktrace(_IO_FILE*) [out/Debug/node]
12:21:26      2: 0xaaaaebd37cbc  [out/Debug/node]
12:21:26      3: 0xaaaaebd37ce8  [out/Debug/node]
12:21:26      4: 0xaaaaede4b9e4 V8_Fatal(char const*, int, char const*, ...) [out/Debug/node]
12:21:26      5: 0xaaaaede4ba28  [out/Debug/node]
12:21:26      6: 0xaaaaec02b82c v8::Object::GetRealNamedProperty(v8::Local<v8::Context>, v8::Local<v8::Name>) [out/Debug/node]
12:21:26      7: 0xaaaaebc3b5a8 node::contextify::ContextifyContext::PropertyGetterCallback(v8::Local<v8::Name>, v8::PropertyCallbackInfo<v8::Value> const&) [out/Debug/node]
12:21:26      8: 0xaaaaec63e724 v8::internal::PropertyCallbackArguments::CallNamedGetter(v8::internal::Handle<v8::internal::InterceptorInfo>, v8::internal::Handle<v8::internal::Name>) [out/Debug/node]
12:21:26      9: 0xaaaaec8e0b44  [out/Debug/node]
12:21:26     10: 0xaaaaeca08494 v8::internal::Object::GetProperty(v8::internal::LookupIterator*, bool) [out/Debug/node]
12:21:26     11: 0xaaaaec64e2ac v8::internal::LoadIC::Load(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>, bool, v8::internal::Handle<v8::internal::Object>) [out/Debug/node]
12:21:26     12: 0xaaaaec64e8ec v8::internal::Runtime_LoadNoFeedbackIC_Miss(int, unsigned long*, v8::internal::Isolate*) [out/Debug/node]
12:21:26     13: 0xaaaaed451e74  [out/Debug/node]
12:21:26   ...

@targos
Copy link
Member Author

targos commented Dec 22, 2023

Test is from nodejs/node#27523

/cc @addaleax @nodejs/cpp-reviewers

@deepak1556
Copy link

We also hit these checks in our latest roll electron/electron@dc65d34. Turned out to be a required change on the embedder side from https://chromium-review.googlesource.com/c/v8/v8/+/5050065. I only patched the callsites that triggered failing tests but there might be cases that are not exercised in the tests. IIUC, V8 now drops the embedder side exceptions before re-entering and which would require explicit catch block.

@codebytere
Copy link
Member

codebytere commented Jan 2, 2024

The above changes also trigger termination DCHECKs seemingly nondeterministically unless we add

diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc
index 69e2a389f9e1480a1a4ba37f5df5356b42f7d52d..0c29b00298b44b97f88a63aa5b89f1c201f6326a 100644
--- a/src/handle_wrap.cc
+++ b/src/handle_wrap.cc
@@ -148,6 +148,9 @@ void HandleWrap::OnClose(uv_handle_t* handle) {
   wrap->OnClose();
   wrap->handle_wrap_queue_.Remove();

+  if (env->isolate()->IsExecutionTerminating())
+    return;
+
   if (!wrap->persistent().IsEmpty() &&
       wrap->object()->Has(env->context(), env->handle_onclose_symbol())
       .FromMaybe(false)) {

This seems mostly to happen on Linux (can't repro on macOS) and to betwen 2-8 es-module tests depending on the run.

@targos
Copy link
Member Author

targos commented Jan 7, 2024

Thank you @deepak1556 I included your changes in nodejs/node@9570832

@codebytere By the above changes you mean the added TryCatch? Is the diff you propose a workaround or the correct fix?

@targos
Copy link
Member Author

targos commented Jan 7, 2024

I'm closing this issue and suggest to continue discussion in nodejs/node#51362

@targos targos closed this as completed Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants