From a96d5d1bccc66978b1a6147e261f194cf3eede0a Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 7 Sep 2024 07:31:19 -0700 Subject: [PATCH] src: move more stuff over to use Maybe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/54831 Reviewed-By: Yagiz Nizipli Reviewed-By: Michaël Zasso --- src/api/environment.cc | 42 +++++++++++++++++++++++------------------- src/api/hooks.cc | 4 ++-- src/base_object.cc | 5 +++-- src/base_object.h | 2 +- src/cares_wrap.cc | 8 ++++---- src/node_env_var.cc | 7 +++---- src/node_internals.h | 6 +++--- src/node_messaging.cc | 19 ++++++++++++------- src/node_messaging.h | 2 +- src/spawn_sync.cc | 24 ++++++++++++------------ src/spawn_sync.h | 2 +- src/util.h | 2 +- 12 files changed, 66 insertions(+), 57 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 77c20a4b6b9db4..6cdc93614515db 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -31,6 +31,7 @@ using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Isolate; using v8::Just; +using v8::JustVoid; using v8::Local; using v8::Maybe; using v8::MaybeLocal; @@ -635,7 +636,7 @@ void ProtoThrower(const FunctionCallbackInfo& info) { // This runs at runtime, regardless of whether the context // is created from a snapshot. -Maybe InitializeContextRuntime(Local context) { +Maybe InitializeContextRuntime(Local context) { Isolate* isolate = context->GetIsolate(); HandleScope handle_scope(isolate); @@ -653,7 +654,7 @@ Maybe InitializeContextRuntime(Local context) { Boolean::New(isolate, is_code_generation_from_strings_allowed)); if (per_process::cli_options->disable_proto == "") { - return Just(true); + return JustVoid(); } // Remove __proto__ @@ -669,14 +670,14 @@ Maybe InitializeContextRuntime(Local context) { if (!context->Global() ->Get(context, object_string) .ToLocal(&object_v)) { - return Nothing(); + return Nothing(); } Local prototype_v; if (!object_v.As() ->Get(context, prototype_string) .ToLocal(&prototype_v)) { - return Nothing(); + return Nothing(); } prototype = prototype_v.As(); @@ -689,13 +690,13 @@ Maybe InitializeContextRuntime(Local context) { if (prototype ->Delete(context, proto_string) .IsNothing()) { - return Nothing(); + return Nothing(); } } else if (per_process::cli_options->disable_proto == "throw") { Local thrower; if (!Function::New(context, ProtoThrower) .ToLocal(&thrower)) { - return Nothing(); + return Nothing(); } PropertyDescriptor descriptor(thrower, thrower); @@ -704,17 +705,17 @@ Maybe InitializeContextRuntime(Local context) { if (prototype ->DefineProperty(context, proto_string, descriptor) .IsNothing()) { - return Nothing(); + return Nothing(); } } else if (per_process::cli_options->disable_proto != "") { // Validated in ProcessGlobalArgs UNREACHABLE("invalid --disable-proto mode"); } - return Just(true); + return JustVoid(); } -Maybe InitializeBaseContextForSnapshot(Local context) { +Maybe InitializeBaseContextForSnapshot(Local context) { Isolate* isolate = context->GetIsolate(); HandleScope handle_scope(isolate); @@ -728,18 +729,18 @@ Maybe InitializeBaseContextForSnapshot(Local context) { Local intl_v; if (!context->Global()->Get(context, intl_string).ToLocal(&intl_v)) { - return Nothing(); + return Nothing(); } if (intl_v->IsObject() && intl_v.As()->Delete(context, break_iter_string).IsNothing()) { - return Nothing(); + return Nothing(); } } - return Just(true); + return JustVoid(); } -Maybe InitializeMainContextForSnapshot(Local context) { +Maybe InitializeMainContextForSnapshot(Local context) { Isolate* isolate = context->GetIsolate(); HandleScope handle_scope(isolate); @@ -750,12 +751,12 @@ Maybe InitializeMainContextForSnapshot(Local context) { ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate)); if (InitializeBaseContextForSnapshot(context).IsNothing()) { - return Nothing(); + return Nothing(); } return InitializePrimordials(context); } -Maybe InitializePrimordials(Local context) { +Maybe InitializePrimordials(Local context) { // Run per-context JS files. Isolate* isolate = context->GetIsolate(); Context::Scope context_scope(context); @@ -769,7 +770,7 @@ Maybe InitializePrimordials(Local context) { if (primordials->SetPrototype(context, Null(isolate)).IsNothing() || !GetPerContextExports(context).ToLocal(&exports) || exports->Set(context, primordials_string, primordials).IsNothing()) { - return Nothing(); + return Nothing(); } static const char* context_files[] = {"internal/per_context/primordials", @@ -793,11 +794,11 @@ Maybe InitializePrimordials(Local context) { context, *module, arraysize(arguments), arguments, nullptr) .IsEmpty()) { // Execution failed during context creation. - return Nothing(); + return Nothing(); } } - return Just(true); + return JustVoid(); } // This initializes the main context (i.e. vm contexts are not included). @@ -806,7 +807,10 @@ Maybe InitializeContext(Local context) { return Nothing(); } - return InitializeContextRuntime(context); + if (InitializeContextRuntime(context).IsNothing()) { + return Nothing(); + } + return Just(true); } uv_loop_t* GetCurrentEventLoop(Isolate* isolate) { diff --git a/src/api/hooks.cc b/src/api/hooks.cc index 65d39e6b9ff921..cf95d009d4a088 100644 --- a/src/api/hooks.cc +++ b/src/api/hooks.cc @@ -46,8 +46,8 @@ Maybe EmitProcessBeforeExit(Environment* env) { Local exit_code = Integer::New( isolate, static_cast(env->exit_code(ExitCode::kNoFailure))); - return ProcessEmit(env, "beforeExit", exit_code).IsEmpty() ? - Nothing() : Just(true); + return ProcessEmit(env, "beforeExit", exit_code).IsEmpty() ? Nothing() + : Just(true); } static ExitCode EmitExitInternal(Environment* env) { diff --git a/src/base_object.cc b/src/base_object.cc index 58ceecca2f91d7..c8798fc572d389 100644 --- a/src/base_object.cc +++ b/src/base_object.cc @@ -10,6 +10,7 @@ using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; using v8::Just; +using v8::JustVoid; using v8::Local; using v8::Maybe; using v8::Object; @@ -110,9 +111,9 @@ Maybe>> BaseObject::NestedTransferables() return Just(std::vector>{}); } -Maybe BaseObject::FinalizeTransferRead(Local context, +Maybe BaseObject::FinalizeTransferRead(Local context, ValueDeserializer* deserializer) { - return Just(true); + return JustVoid(); } BaseObject::PointerData* BaseObject::pointer_data() { diff --git a/src/base_object.h b/src/base_object.h index 546d968e5ca424..cbf27190f135e7 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -173,7 +173,7 @@ class BaseObject : public MemoryRetainer { virtual std::unique_ptr CloneForMessaging() const; virtual v8::Maybe>> NestedTransferables() const; - virtual v8::Maybe FinalizeTransferRead( + virtual v8::Maybe FinalizeTransferRead( v8::Local context, v8::ValueDeserializer* deserializer); // Indicates whether this object is expected to use a strong reference during diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index c4510bdf63b276..4f87062d348d2b 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1480,13 +1480,13 @@ void AfterGetAddrInfo(uv_getaddrinfo_t* req, int status, struct addrinfo* res) { switch (order) { case DNS_ORDER_IPV4_FIRST: - if (add(true, false).IsNothing()) return; - if (add(false, true).IsNothing()) return; + if (add(true, false).IsNothing() || add(false, true).IsNothing()) + return; break; case DNS_ORDER_IPV6_FIRST: - if (add(false, true).IsNothing()) return; - if (add(true, false).IsNothing()) return; + if (add(false, true).IsNothing() || add(true, false).IsNothing()) + return; break; default: diff --git a/src/node_env_var.cc b/src/node_env_var.cc index fdf86f17d460f5..d19d11dc714e08 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -19,7 +19,6 @@ using v8::HandleScope; using v8::Integer; using v8::Intercepted; using v8::Isolate; -using v8::Just; using v8::JustVoid; using v8::Local; using v8::Maybe; @@ -320,7 +319,7 @@ Maybe KVStore::AssignFromObject(Local context, // TODO(bnoordhuis) Not super efficient but called infrequently. Not worth // the trouble yet of specializing for RealEnvStore and MapKVStore. -Maybe KVStore::AssignToObject(v8::Isolate* isolate, +Maybe KVStore::AssignToObject(v8::Isolate* isolate, v8::Local context, v8::Local object) { HandleScope scope(isolate); @@ -333,9 +332,9 @@ Maybe KVStore::AssignToObject(v8::Isolate* isolate, ok = ok && key->IsString(); ok = ok && Get(isolate, key.As()).ToLocal(&value); ok = ok && object->Set(context, key, value).To(&ok); - if (!ok) return Nothing(); + if (!ok) return Nothing(); } - return Just(true); + return JustVoid(); } static Intercepted EnvGetter(Local property, diff --git a/src/node_internals.h b/src/node_internals.h index fe2d25decd8830..85b666e11f5654 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -110,10 +110,10 @@ void SignalExit(int signal, siginfo_t* info, void* ucontext); std::string GetProcessTitle(const char* default_title); std::string GetHumanReadableProcessName(); -v8::Maybe InitializeBaseContextForSnapshot( +v8::Maybe InitializeBaseContextForSnapshot( v8::Local context); -v8::Maybe InitializeContextRuntime(v8::Local context); -v8::Maybe InitializePrimordials(v8::Local context); +v8::Maybe InitializeContextRuntime(v8::Local context); +v8::Maybe InitializePrimordials(v8::Local context); class NodeArrayBufferAllocator : public ArrayBufferAllocator { public: diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 7422757fcb21bf..57b051da935b31 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -25,6 +25,7 @@ using v8::Global; using v8::HandleScope; using v8::Isolate; using v8::Just; +using v8::JustVoid; using v8::Local; using v8::Maybe; using v8::MaybeLocal; @@ -337,7 +338,11 @@ class SerializerDelegate : public ValueSerializer::Delegate { // methods like toString(). It's probably confusing if that gets lost // in transmission. Local normal_object = Object::New(isolate); - env_->env_vars()->AssignToObject(isolate, env_->context(), normal_object); + if (env_->env_vars() + ->AssignToObject(isolate, env_->context(), normal_object) + .IsNothing()) { + return Nothing(); + } serializer->WriteUint32(kNormalObject); // Instead of a BaseObject. return serializer->WriteValue(env_->context(), normal_object); } @@ -1389,25 +1394,25 @@ JSTransferable::NestedTransferables() const { return Just(ret); } -Maybe JSTransferable::FinalizeTransferRead( +Maybe JSTransferable::FinalizeTransferRead( Local context, ValueDeserializer* deserializer) { // Call `this[kDeserialize](data)` where `data` comes from the return value // of `this[kTransfer]()` or `this[kClone]()`. HandleScope handle_scope(env()->isolate()); Local data; - if (!deserializer->ReadValue(context).ToLocal(&data)) return Nothing(); + if (!deserializer->ReadValue(context).ToLocal(&data)) return Nothing(); Local method_name = env()->messaging_deserialize_symbol(); Local method; if (!target()->Get(context, method_name).ToLocal(&method)) { - return Nothing(); + return Nothing(); } - if (!method->IsFunction()) return Just(true); + if (!method->IsFunction()) return JustVoid(); if (method.As()->Call(context, target(), 1, &data).IsEmpty()) { - return Nothing(); + return Nothing(); } - return Just(true); + return JustVoid(); } JSTransferable::Data::Data(std::string&& deserialize_info, diff --git a/src/node_messaging.h b/src/node_messaging.h index 6dd34b8bd5a407..169ff0ba19e909 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -340,7 +340,7 @@ class JSTransferable : public BaseObject { std::unique_ptr CloneForMessaging() const override; v8::Maybe>> NestedTransferables() const override; - v8::Maybe FinalizeTransferRead( + v8::Maybe FinalizeTransferRead( v8::Local context, v8::ValueDeserializer* deserializer) override; diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 36570d069ad00b..5f20e9cc0881f9 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -41,6 +41,7 @@ using v8::Int32; using v8::Integer; using v8::Isolate; using v8::Just; +using v8::JustVoid; using v8::Local; using v8::Maybe; using v8::MaybeLocal; @@ -441,7 +442,7 @@ MaybeLocal SyncProcessRunner::Run(Local options) { CHECK_EQ(lifecycle_, kUninitialized); - Maybe r = TryInitializeAndRunLoop(options); + Maybe r = TryInitializeAndRunLoop(options); CloseHandlesAndDeleteLoop(); if (r.IsNothing()) return MaybeLocal(); @@ -450,7 +451,7 @@ MaybeLocal SyncProcessRunner::Run(Local options) { return scope.Escape(result); } -Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { +Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { int r; // There is no recovery from failure inside TryInitializeAndRunLoop - the @@ -461,7 +462,7 @@ Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { uv_loop_ = new uv_loop_t; if (uv_loop_ == nullptr) { SetError(UV_ENOMEM); - return Just(false); + return JustVoid(); } r = uv_loop_init(uv_loop_); @@ -469,21 +470,21 @@ Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { delete uv_loop_; uv_loop_ = nullptr; SetError(r); - return Just(false); + return JustVoid(); } - if (!ParseOptions(options).To(&r)) return Nothing(); + if (!ParseOptions(options).To(&r)) return Nothing(); if (r < 0) { SetError(r); - return Just(false); + return JustVoid(); } if (timeout_ > 0) { r = uv_timer_init(uv_loop_, &uv_timer_); if (r < 0) { SetError(r); - return Just(false); + return JustVoid(); } uv_unref(reinterpret_cast(&uv_timer_)); @@ -498,7 +499,7 @@ Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { r = uv_timer_start(&uv_timer_, KillTimerCallback, timeout_, 0); if (r < 0) { SetError(r); - return Just(false); + return JustVoid(); } } @@ -506,7 +507,7 @@ Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { r = uv_spawn(uv_loop_, &uv_process_, &uv_process_options_); if (r < 0) { SetError(r); - return Just(false); + return JustVoid(); } uv_process_.data = this; @@ -515,7 +516,7 @@ Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { r = pipe->Start(); if (r < 0) { SetPipeError(r); - return Just(false); + return JustVoid(); } } } @@ -527,10 +528,9 @@ Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { // If we get here the process should have exited. CHECK_GE(exit_status_, 0); - return Just(true); + return JustVoid(); } - void SyncProcessRunner::CloseHandlesAndDeleteLoop() { CHECK_LT(lifecycle_, kHandlesClosed); diff --git a/src/spawn_sync.h b/src/spawn_sync.h index d5b74e67d83094..8243737130af1b 100644 --- a/src/spawn_sync.h +++ b/src/spawn_sync.h @@ -155,7 +155,7 @@ class SyncProcessRunner { inline Environment* env() const; v8::MaybeLocal Run(v8::Local options); - v8::Maybe TryInitializeAndRunLoop(v8::Local options); + v8::Maybe TryInitializeAndRunLoop(v8::Local options); void CloseHandlesAndDeleteLoop(); void CloseStdioPipes(); diff --git a/src/util.h b/src/util.h index 792108317ea594..571fe2b378ee7b 100644 --- a/src/util.h +++ b/src/util.h @@ -323,7 +323,7 @@ class KVStore { virtual std::shared_ptr Clone(v8::Isolate* isolate) const; virtual v8::Maybe AssignFromObject(v8::Local context, v8::Local entries); - v8::Maybe AssignToObject(v8::Isolate* isolate, + v8::Maybe AssignToObject(v8::Isolate* isolate, v8::Local context, v8::Local object);