From be8fbda3a3097f1507ea28dd483c94ca97ce9b44 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Sun, 26 Jun 2022 04:22:45 +0900 Subject: [PATCH 1/4] src,stream: change return type to `Maybe` This changes the return types of some functions to indicate that the functions may have a pending exception, and removes some of todos related. Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- src/stream_wrap.cc | 7 +++---- src/stream_wrap.h | 4 +--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index ef85ba681f8588..a2c2a92008f2d4 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -239,8 +239,7 @@ static MaybeLocal AcceptHandle(Environment* env, return scope.Escape(wrap_obj); } - -void LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) { +v8::Maybe LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) { HandleScope scope(env()->isolate()); Context::Scope context_scope(env()->context()); uv_handle_type type = UV_UNKNOWN_HANDLE; @@ -272,14 +271,14 @@ void LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) { object()->Set(env()->context(), env()->pending_handle_string(), local_pending_obj).IsNothing()) { - return; + return v8::Nothing(); } } EmitRead(nread, *buf); + return v8::JustVoid(); } - void LibuvStreamWrap::GetWriteQueueSize( const FunctionCallbackInfo& info) { LibuvStreamWrap* wrap; diff --git a/src/stream_wrap.h b/src/stream_wrap.h index 8cbce923bfed89..197303990c90bc 100644 --- a/src/stream_wrap.h +++ b/src/stream_wrap.h @@ -105,9 +105,7 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase { // Callbacks for libuv void OnUvAlloc(size_t suggested_size, uv_buf_t* buf); - // TODO(RaisinTen): Update the return type to a Maybe, so that we can indicate - // if there is a pending exception/termination. - void OnUvRead(ssize_t nread, const uv_buf_t* buf); + v8::Maybe OnUvRead(ssize_t nread, const uv_buf_t* buf); static void AfterUvWrite(uv_write_t* req, int status); static void AfterUvShutdown(uv_shutdown_t* req, int status); From 8a7c26ff3e7fd1b9b24e667030b773736dc96a4f Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Sun, 26 Jun 2022 22:30:33 +0900 Subject: [PATCH 2/4] fixup: create using declarations Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- src/stream_wrap.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index a2c2a92008f2d4..6fe89f641e6a48 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -44,8 +44,11 @@ using v8::EscapableHandleScope; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; +using v8::JustVoid; using v8::Local; +using v8::Maybe; using v8::MaybeLocal; +using v8::Nothing; using v8::Object; using v8::PropertyAttribute; using v8::ReadOnly; @@ -239,7 +242,7 @@ static MaybeLocal AcceptHandle(Environment* env, return scope.Escape(wrap_obj); } -v8::Maybe LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) { +Maybe LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) { HandleScope scope(env()->isolate()); Context::Scope context_scope(env()->context()); uv_handle_type type = UV_UNKNOWN_HANDLE; @@ -271,12 +274,12 @@ v8::Maybe LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) { object()->Set(env()->context(), env()->pending_handle_string(), local_pending_obj).IsNothing()) { - return v8::Nothing(); + return Nothing(); } } EmitRead(nread, *buf); - return v8::JustVoid(); + return JustVoid(); } void LibuvStreamWrap::GetWriteQueueSize( From 83cafe85a70ee0774304ee565ff53d13e0628ca8 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Mon, 27 Jun 2022 21:36:34 +0900 Subject: [PATCH 3/4] fixup: check the uv_handle_type Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- src/stream_wrap.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 6fe89f641e6a48..dd2b9f84438a6d 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -270,10 +270,13 @@ Maybe LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) { } Local local_pending_obj; - if (pending_obj.ToLocal(&local_pending_obj) && - object()->Set(env()->context(), - env()->pending_handle_string(), - local_pending_obj).IsNothing()) { + if (type != UV_UNKNOWN_HANDLE && + (!pending_obj.ToLocal(&local_pending_obj) || + object() + ->Set(env()->context(), + env()->pending_handle_string(), + local_pending_obj) + .IsNothing())) { return Nothing(); } } From c0e0bea205d033c00c4ced7e0ccfc0d523ac02d3 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Wed, 29 Jun 2022 19:37:39 +0900 Subject: [PATCH 4/4] fixup: add TryCatchScope Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- src/stream_wrap.cc | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index dd2b9f84438a6d..d0c5664adcd897 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -25,6 +25,7 @@ #include "env-inl.h" #include "handle_wrap.h" #include "node_buffer.h" +#include "node_errors.h" #include "node_external_reference.h" #include "pipe_wrap.h" #include "req_wrap-inl.h" @@ -38,6 +39,7 @@ namespace node { +using errors::TryCatchScope; using v8::Context; using v8::DontDelete; using v8::EscapableHandleScope; @@ -194,15 +196,19 @@ bool LibuvStreamWrap::IsIPCPipe() { return is_named_pipe_ipc(); } - int LibuvStreamWrap::ReadStart() { - return uv_read_start(stream(), [](uv_handle_t* handle, - size_t suggested_size, - uv_buf_t* buf) { - static_cast(handle->data)->OnUvAlloc(suggested_size, buf); - }, [](uv_stream_t* stream, ssize_t nread, const uv_buf_t* buf) { - static_cast(stream->data)->OnUvRead(nread, buf); - }); + return uv_read_start( + stream(), + [](uv_handle_t* handle, size_t suggested_size, uv_buf_t* buf) { + static_cast(handle->data) + ->OnUvAlloc(suggested_size, buf); + }, + [](uv_stream_t* stream, ssize_t nread, const uv_buf_t* buf) { + LibuvStreamWrap* wrap = static_cast(stream->data); + TryCatchScope try_catch(wrap->env()); + try_catch.SetVerbose(true); + wrap->OnUvRead(nread, buf); + }); }