From df97c264f38ab5141f34189f591ff59a7dd8e5dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sun, 7 Jan 2024 15:02:32 +0100 Subject: [PATCH] src: add missing TryCatch Otherwise re-entering V8 doesn't work as expected after exceptions were thrown. Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5050065 Co-Authored-By: Toon Verwaest Co-Authored-By: deepak1556 PR-URL: https://github.com/nodejs/node/pull/51362 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: Rafael Gonzaga --- src/js_stream.cc | 7 +++++++ src/node_contextify.cc | 6 ++++++ src/node_messaging.cc | 4 ++++ test/addons/make-callback-recurse/binding.cc | 5 +++++ 4 files changed, 22 insertions(+) diff --git a/src/js_stream.cc b/src/js_stream.cc index 0d13066f54cf00..55cbdf5bf2b0a3 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -20,6 +20,7 @@ using v8::Int32; using v8::Isolate; using v8::Local; using v8::Object; +using v8::TryCatch; using v8::Value; @@ -169,6 +170,8 @@ void JSStream::ReadBuffer(const FunctionCallbackInfo& args) { const char* data = buffer.data(); int len = buffer.length(); + TryCatch try_catch(args.GetIsolate()); + // Repeatedly ask the stream's owner for memory, copy the data that we // just read from JS into those buffers and emit them as reads. while (len != 0) { @@ -182,6 +185,10 @@ void JSStream::ReadBuffer(const FunctionCallbackInfo& args) { len -= static_cast(avail); wrap->EmitRead(avail, buf); } + + if (try_catch.HasCaught()) { + try_catch.ReThrow(); + } } diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 0a409c5086f713..3d0c360b024a95 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -489,6 +489,7 @@ bool ContextifyContext::IsStillInitializing(const ContextifyContext* ctx) { void ContextifyContext::PropertyGetterCallback( Local property, const PropertyCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing @@ -496,6 +497,8 @@ void ContextifyContext::PropertyGetterCallback( Local context = ctx->context(); Local sandbox = ctx->sandbox(); + + TryCatchScope try_catch(env); MaybeLocal maybe_rv = sandbox->GetRealNamedProperty(context, property); if (maybe_rv.IsEmpty()) { @@ -505,6 +508,9 @@ void ContextifyContext::PropertyGetterCallback( Local rv; if (maybe_rv.ToLocal(&rv)) { + if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + try_catch.ReThrow(); + } if (rv == sandbox) rv = ctx->global_proxy(); diff --git a/src/node_messaging.cc b/src/node_messaging.cc index e7d2bfbafef13f..bcc090f585e57a 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -907,6 +907,7 @@ Maybe MessagePort::PostMessage(Environment* env, const TransferList& transfer_v) { Isolate* isolate = env->isolate(); Local obj = object(isolate); + TryCatchScope try_catch(env); std::shared_ptr msg = std::make_shared(); @@ -915,6 +916,9 @@ Maybe MessagePort::PostMessage(Environment* env, Maybe serialization_maybe = msg->Serialize(env, context, message_v, transfer_v, obj); + if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + try_catch.ReThrow(); + } if (data_ == nullptr) { return serialization_maybe; } diff --git a/test/addons/make-callback-recurse/binding.cc b/test/addons/make-callback-recurse/binding.cc index 3fe3212ee3c8f5..145247954a31c4 100644 --- a/test/addons/make-callback-recurse/binding.cc +++ b/test/addons/make-callback-recurse/binding.cc @@ -8,6 +8,7 @@ using v8::FunctionCallbackInfo; using v8::Isolate; using v8::Local; using v8::Object; +using v8::TryCatch; using v8::Value; namespace { @@ -19,8 +20,12 @@ void MakeCallback(const FunctionCallbackInfo& args) { Local recv = args[0].As(); Local method = args[1].As(); + TryCatch try_catch(isolate); node::MakeCallback(isolate, recv, method, 0, nullptr, node::async_context{0, 0}); + if (try_catch.HasCaught()) { + try_catch.ReThrow(); + } } void Initialize(Local exports) {