Panics should not unwind into non-Rust code

[solson]

I just noticed and investigated something in the output in denoland/deno#7953 that I somehow missed the first time around, after the usual panic message:

fatal runtime error: failed to initiate panic, error 5
fish: “deno” terminated by signal SIGABRT (Abort)

A normal Rust panic does not SIGABRT. With the standard panic handler, first they print the panic message and backtrace, and then they try to unwind. That is where this panic aborts, because it happens inside a callback called by V8 and the unwinder gets confused by outer V8 stack frames. (In fact, it may be getting confused specifically by V8 JIT frames that don't show up in the gdb output.)

click for gdb backtrace (13 is C++ which calls 12 which is Rust)

(gdb) bt
#0  0x00007fceaaa3808a in raise () from /nix/store/9df65igwjmf2wbw0gbrrgair6piqjgmi-glibc-2.31/lib/libc.so.6
denoland/deno#1  0x00007fceaaa22528 in abort () from /nix/store/9df65igwjmf2wbw0gbrrgair6piqjgmi-glibc-2.31/lib/libc.so.6
denoland/deno#2  0x00005653a2f605a7 in std::sys::unix::abort_internal () at library/std/src/sys/unix/mod.rs:167
denoland/deno#3  0x00005653a2f60595 in std::sys_common::util::abort () at library/std/src/sys_common/util.rs:19
denoland/deno#4  0x00005653a2f6053d in std::panicking::rust_panic () at library/std/src/panicking.rs:620
denoland/deno#5  0x00005653a2f603ba in std::panicking::rust_panic_with_hook () at library/std/src/panicking.rs:588
denoland/deno#6  0x00005653a2f5ff58 in std::panicking::begin_panic_handler::{{closure}} () at library/std/src/panicking.rs:476
denoland/deno#7  0x00005653a2f5ff24 in std::sys_common::backtrace::__rust_end_short_backtrace () at library/std/src/sys_common/backtrace.rs:153
denoland/deno#8  0x00005653a2f5fedd in rust_begin_unwind () at library/std/src/panicking.rs:475
denoland/deno#9  0x00005653a2c20210 in core::panicking::panic_fmt () at library/core/src/panicking.rs:85
denoland/deno#10 0x00005653a2c272e2 in core::option::expect_none_failed () at library/core/src/option.rs:1221
denoland/deno#11 0x00005653a2a88d85 in core::result::Result<T,E>::unwrap ()
denoland/deno#12 0x00005653a2b8ad18 in <deno::inspector::InspectorSession as rusty_v8::inspector::ChannelImpl>::send_response ()
denoland/deno#13 0x00005653a34e2a77 in non-virtual thunk to v8_inspector::V8InspectorSessionImpl::SendProtocolResponse(int, std::__1::unique_ptr<v8_crdtp::Serializable, std::__1::default_delete<v8_crdtp::Serializable> >) ()
denoland/deno#14 0x00005653a34faa21 in v8_crdtp::DomainDispatcher::sendResponse(int, v8_crdtp::DispatchResponse const&, std::__1::unique_ptr<v8_crdtp::Serializable, std::__1::default_delete<v8_crdtp::Serializable> >) ()
    at ../../../../v8/third_party/inspector_protocol/crdtp/dispatch.cc:473
denoland/deno#15 0x00005653a34fa890 in v8_crdtp::DomainDispatcher::Callback::sendIfActive(std::__1::unique_ptr<v8_crdtp::Serializable, std::__1::default_delete<v8_crdtp::Serializable> >, v8_crdtp::DispatchResponse const&) ()
    at ../../../../v8/third_party/inspector_protocol/crdtp/dispatch.cc:442
denoland/deno#16 0x00005653a39a84a7 in v8_inspector::protocol::Runtime::EvaluateCallbackImpl::sendSuccess(std::__1::unique_ptr<v8_inspector::protocol::Runtime::RemoteObject, std::__1::default_delete<v8_inspector::protocol::Runtime::RemoteObject> >, v8_crdtp::detail::PtrMaybe<v8_inspector::protocol::Runtime::ExceptionDetails>) () at gen/v8/src/inspector/protocol/Runtime.cpp:879
denoland/deno#17 0x00005653a34f1245 in v8_inspector::(anonymous namespace)::EvaluateCallbackWrapper<v8_inspector::protocol::Runtime::Backend::EvaluateCallback>::sendSuccess(std::__1::unique_ptr<v8_inspector::protocol::Runtime::RemoteObject, std::__1::default_delete<v8_inspector::protocol::Runtime::RemoteObject> >, v8_crdtp::detail::PtrMaybe<v8_inspector::protocol::Runtime::ExceptionDetails>) () at ../../../../v8/src/inspector/v8-runtime-agent-impl.cc:76
denoland/deno#18 0x00005653a39b1761 in v8_inspector::InjectedScript::ProtocolPromiseHandler::thenCallback(v8::Local<v8::Value>) () at ../../../../v8/src/inspector/injected-script.cc:217
denoland/deno#19 0x00005653a39b0f8b in v8_inspector::InjectedScript::ProtocolPromiseHandler::thenCallback(v8::FunctionCallbackInfo<v8::Value> const&) () at ../../../../v8/src/inspector/injected-script.cc:125
denoland/deno#20 0x00005653a376d1a9 in v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) () at ../../../../v8/src/api/api-arguments-inl.h:158
denoland/deno#21 0x00005653a376c6c0 in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) () at ../../../../v8/src/builtins/builtins-api.cc:111
denoland/deno#22 0x00005653a376bc9b in v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) () at ../../../../v8/src/builtins/builtins-api.cc:141
denoland/deno#23 0x00005653a36306b8 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit ()
denoland/deno#24 0x00005653a367fbbb in Builtins_PromiseFulfillReactionJob ()
denoland/deno#25 0x00005653a35e3037 in Builtins_RunMicrotasks ()
denoland/deno#26 0x00005653a35c07f8 in Builtins_JSRunMicrotasksEntry ()
denoland/deno#27 0x00005653a31a114f in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) () at ../../../../v8/src/execution/execution.cc:383
denoland/deno#28 0x00005653a31a17e0 in v8::internal::(anonymous namespace)::InvokeWithTryCatch(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) () at ../../../../v8/src/execution/execution.cc:428
denoland/deno#29 0x00005653a31a18de in v8::internal::Execution::TryRunMicrotasks(v8::internal::Isolate*, v8::internal::MicrotaskQueue*, v8::internal::MaybeHandle<v8::internal::Object>*) () at ../../../../v8/src/execution/execution.cc:505
denoland/deno#30 0x00005653a31c9405 in RunMicrotasks () at ../../../../v8/src/execution/microtask-queue.cc:165
denoland/deno#31 0x00005653a31c9227 in v8::internal::MicrotaskQueue::PerformCheckpoint(v8::Isolate*) () at ../../../../v8/src/execution/microtask-queue.cc:117
denoland/deno#32 0x00005653a31b8e38 in v8::internal::Isolate::FireCallCompletedCallback(v8::internal::MicrotaskQueue*) () at ../../../../v8/src/execution/isolate.cc:4066
denoland/deno#33 0x00005653a312e670 in v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) () at ../../../../v8/src/api/api.cc:4993
denoland/deno#34 0x00005653a2f383f2 in rusty_v8::function::<impl rusty_v8::data::Function>::call ()
denoland/deno#35 0x00005653a2bc346a in <deno::worker::Worker as core::future::future::Future>::poll ()
denoland/deno#36 0x00005653a2aeeae3 in <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll ()
denoland/deno#37 0x00005653a2ba9475 in deno::repl::run::{{closure}} ()
#38 0x00005653a2af9a68 in <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll ()
denoland/deno#39 0x00005653a2ab867b in tokio::runtime::Runtime::block_on ()
denoland/deno#40 0x00005653a2bcf39d in deno::main ()
denoland/deno#41 0x00005653a2a5fb48 in std::sys_common::backtrace::__rust_begin_short_backtrace ()
denoland/deno#42 0x00005653a2bd62ef in main ()

As I understand it, we should try to prevent ever unwinding into non-Rust stack frames by making callbacks like this one use catch_unwind and then either manually exit or convert the error into something that can be sensibly passed up into V8, whatever is most appropriate in each case. This should be done wherever Rust code is called from non-Rust code in Deno and its dependencies.

Quoting the catch_unwind docs:

It is currently undefined behavior to unwind from Rust code into foreign code, so this function is particularly useful when Rust is called from another language (normally C). This can run arbitrary Rust code, capturing a panic and allowing a graceful handling of the error.

[bartlomieju]

@bnoordhuis @piscisaureus @ry WDYT?

[GGICE]

+1

[ry]

Yes I think so too. Tho I think this should probably be dealt with at the rusty v8 layer...

[bnoordhuis]

I'll move this to rusty_v8. The analysis that unwinding can't cross over into C++ or JS stack frames is correct, I'm just not sure how to handle that except abort. I don't think it's always possible to tell V8 to unwind until we're back in Rust territory again.