From 2c2892bf88f43cae1a0a017cc1984b504033e666 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Fri, 8 Sep 2023 13:08:09 +0200
Subject: [PATCH] src: set ModuleWrap internal fields only once

There is no need to initialize the internal fields to undefined
and then initialize them to something else in the caller. Simply
pass the internal fields into the constructor to initialize
them just once.

PR-URL: https://github.com/nodejs/node/pull/49391
Backport-PR-URL: https://github.com/nodejs/node/pull/51004
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
---
 src/module_wrap.cc | 40 +++++++++++++++++++++++-----------------
 src/module_wrap.h  |  4 +++-
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index 2e3d444f64cd15..a1b7633a31d1df 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -52,16 +52,22 @@ using v8::Value;
 ModuleWrap::ModuleWrap(Environment* env,
                        Local<Object> object,
                        Local<Module> module,
-                       Local<String> url)
-  : BaseObject(env, object),
-    module_(env->isolate(), module),
-    id_(env->get_next_module_id()) {
+                       Local<String> url,
+                       Local<Object> context_object,
+                       Local<Value> synthetic_evaluation_step)
+    : BaseObject(env, object),
+      module_(env->isolate(), module),
+      id_(env->get_next_module_id()) {
   env->id_to_module_map.emplace(id_, this);
 
-  Local<Value> undefined = Undefined(env->isolate());
   object->SetInternalField(kURLSlot, url);
-  object->SetInternalField(kSyntheticEvaluationStepsSlot, undefined);
-  object->SetInternalField(kContextObjectSlot, undefined);
+  object->SetInternalField(kSyntheticEvaluationStepsSlot,
+                           synthetic_evaluation_step);
+  object->SetInternalField(kContextObjectSlot, context_object);
+
+  if (!synthetic_evaluation_step->IsUndefined()) {
+    synthetic_ = true;
+  }
 }
 
 ModuleWrap::~ModuleWrap() {
@@ -79,7 +85,9 @@ ModuleWrap::~ModuleWrap() {
 
 Local<Context> ModuleWrap::context() const {
   Local<Value> obj = object()->GetInternalField(kContextObjectSlot).As<Value>();
-  if (obj.IsEmpty()) return {};
+  // If this fails, there is likely a bug e.g. ModuleWrap::context() is accessed
+  // before the ModuleWrap constructor completes.
+  CHECK(obj->IsObject());
   return obj.As<Object>()->GetCreationContext().ToLocalChecked();
 }
 
@@ -227,18 +235,16 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  ModuleWrap* obj = new ModuleWrap(env, that, module, url);
-
-  if (synthetic) {
-    obj->synthetic_ = true;
-    obj->object()->SetInternalField(kSyntheticEvaluationStepsSlot, args[3]);
-  }
-
   // Use the extras object as an object whose GetCreationContext() will be the
   // original `context`, since the `Context` itself strictly speaking cannot
   // be stored in an internal field.
-  obj->object()->SetInternalField(kContextObjectSlot,
-      context->GetExtrasBindingObject());
+  Local<Object> context_object = context->GetExtrasBindingObject();
+  Local<Value> synthetic_evaluation_step =
+      synthetic ? args[3] : Undefined(env->isolate()).As<v8::Value>();
+
+  ModuleWrap* obj = new ModuleWrap(
+      env, that, module, url, context_object, synthetic_evaluation_step);
+
   obj->contextify_context_ = contextify_context;
 
   env->hash_to_module_map.emplace(module->GetIdentityHash(), obj);
diff --git a/src/module_wrap.h b/src/module_wrap.h
index ce4610a461a2b4..00a22b79a801ab 100644
--- a/src/module_wrap.h
+++ b/src/module_wrap.h
@@ -72,7 +72,9 @@ class ModuleWrap : public BaseObject {
   ModuleWrap(Environment* env,
              v8::Local<v8::Object> object,
              v8::Local<v8::Module> module,
-             v8::Local<v8::String> url);
+             v8::Local<v8::String> url,
+             v8::Local<v8::Object> context_object,
+             v8::Local<v8::Value> synthetic_evaluation_step);
   ~ModuleWrap() override;
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);