Skip to content

Commit 438df3b

Browse files
legendecaslegendecas
committed
module: migrate to script context based host defined options
This allows the host-defined options to be an arbitrary JavaScript value so that the module/script wrapper object can be used as the host-defined options instead. In this way, we can move away from the id-based module tables and fixes the problem that the scripts can out lives the module wrapper objects.
1 parent d731a0e commit 438df3b

20 files changed

+156
-189
lines changed

‎lib/internal/modules/esm/create_dynamic_module.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ ${ArrayPrototypeJoin(ArrayPrototypeMap(imports, createImport), '\n')}
3535
${ArrayPrototypeJoin(ArrayPrototypeMap(exports, createExport), '\n')}
3636
import.meta.done();
3737
`;
38-
const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
38+
const { ModuleWrap, moduleMetaSym } = internalBinding('module_wrap');
3939
const m = new ModuleWrap(`${url}`, undefined, source, 0, 0);
4040

4141
const readyfns = new SafeSet();
@@ -47,7 +47,7 @@ import.meta.done();
4747
if (imports.length)
4848
reflect.imports = ObjectCreate(null);
4949

50-
callbackMap.set(m, {
50+
m[moduleMetaSym] = {
5151
initializeImportMeta: (meta, wrap) => {
5252
meta.exports = reflect.exports;
5353
if (reflect.imports)
@@ -61,7 +61,7 @@ import.meta.done();
6161
}
6262
};
6363
},
64-
});
64+
};
6565

6666
return {
6767
module: m,

‎lib/internal/modules/esm/loader.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -362,13 +362,13 @@ class ESMLoader {
362362
url = pathToFileURL(`${process.cwd()}/[eval${++this.evalIndex}]`).href
363363
) {
364364
const evalInstance = (url) => {
365-
const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
365+
const { ModuleWrap, moduleMetaSym } = internalBinding('module_wrap');
366366
const module = new ModuleWrap(url, undefined, source, 0, 0);
367-
callbackMap.set(module, {
367+
module[moduleMetaSym] = {
368368
importModuleDynamically: (specifier, { url }, importAssertions) => {
369369
return this.import(specifier, url, importAssertions);
370370
}
371-
});
371+
};
372372

373373
return module;
374374
};

‎lib/internal/modules/esm/translators.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,8 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
117117
maybeCacheSourceMap(url, source);
118118
debug(`Translating StandardModule ${url}`);
119119
const module = new ModuleWrap(url, undefined, source, 0, 0);
120-
moduleWrap.callbackMap.set(module, {
121-
initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }),
122-
importModuleDynamically,
123-
});
120+
module[moduleWrap.moduleMetaSym].initializeImportMeta = (meta, wrap) => this.importMetaInitialize(meta, { url });
121+
module[moduleWrap.moduleMetaSym].importModuleDynamically = importModuleDynamically;
124122
return module;
125123
});
126124

‎lib/internal/process/esm_loader.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ const {
1818
} = require('internal/vm/module');
1919

2020
exports.initializeImportMetaObject = function(wrap, meta) {
21-
const { callbackMap } = internalBinding('module_wrap');
22-
if (callbackMap.has(wrap)) {
23-
const { initializeImportMeta } = callbackMap.get(wrap);
21+
const { moduleMetaSym } = internalBinding('module_wrap');
22+
if (wrap[moduleMetaSym]) {
23+
const { initializeImportMeta } = wrap[moduleMetaSym];
2424
if (initializeImportMeta !== undefined) {
2525
initializeImportMeta(meta, getModuleFromWrap(wrap) || wrap);
2626
}
@@ -29,9 +29,9 @@ exports.initializeImportMetaObject = function(wrap, meta) {
2929

3030
exports.importModuleDynamicallyCallback =
3131
async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
32-
const { callbackMap } = internalBinding('module_wrap');
33-
if (callbackMap.has(wrap)) {
34-
const { importModuleDynamically } = callbackMap.get(wrap);
32+
const { moduleMetaSym } = internalBinding('module_wrap');
33+
if (wrap[moduleMetaSym]) {
34+
const { importModuleDynamically } = wrap[moduleMetaSym];
3535
if (importModuleDynamically !== undefined) {
3636
return importModuleDynamically(
3737
specifier, getModuleFromWrap(wrap) || wrap, assertions);

‎lib/internal/process/pre_execution.js

-3
Original file line numberDiff line numberDiff line change
@@ -571,9 +571,6 @@ function initializeCJSLoader() {
571571
}
572572

573573
function initializeESMLoader() {
574-
// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
575-
internalBinding('module_wrap').callbackMap = new SafeWeakMap();
576-
577574
if (getEmbedderOptions().shouldNotRegisterESMLoader) return;
578575

579576
const {

‎lib/internal/vm.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,12 @@ function internalCompileFunction(code, params, options) {
9696
'options.importModuleDynamically');
9797
const { importModuleDynamicallyWrap } =
9898
require('internal/vm/module');
99-
const { callbackMap } = internalBinding('module_wrap');
99+
const { moduleMetaSym } = internalBinding('module_wrap');
100100
const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
101101
const func = result.function;
102-
callbackMap.set(result.cacheKey, {
102+
result.cacheKey[moduleMetaSym] = {
103103
importModuleDynamically: (s, _k, i) => wrapped(s, func, i),
104-
});
104+
};
105105
}
106106

107107
return result;

‎lib/internal/vm/module.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,10 @@ class Module {
126126
options.lineOffset, options.columnOffset,
127127
options.cachedData);
128128

129-
binding.callbackMap.set(this[kWrap], {
130-
initializeImportMeta: options.initializeImportMeta,
131-
importModuleDynamically: options.importModuleDynamically ?
129+
this[kWrap][binding.moduleMetaSym].initializeImportMeta = options.initializeImportMeta;
130+
this[kWrap][binding.moduleMetaSym].importModuleDynamically = options.importModuleDynamically ?
132131
importModuleDynamicallyWrap(options.importModuleDynamically) :
133-
undefined,
134-
});
132+
undefined;
135133
} else {
136134
assert(syntheticEvaluationSteps);
137135
this[kWrap] = new ModuleWrap(identifier, context,

‎lib/vm.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,11 @@ class Script extends ContextifyScript {
113113
'options.importModuleDynamically');
114114
const { importModuleDynamicallyWrap } =
115115
require('internal/vm/module');
116-
const { callbackMap } = internalBinding('module_wrap');
117-
callbackMap.set(this, {
116+
const { moduleMetaSym } = internalBinding('module_wrap');
117+
this[moduleMetaSym] = {
118118
importModuleDynamically:
119119
importModuleDynamicallyWrap(importModuleDynamically),
120-
});
120+
};
121121
}
122122
}
123123

‎src/env-inl.h

-10
Original file line numberDiff line numberDiff line change
@@ -389,16 +389,6 @@ inline AliasedInt32Array& Environment::stream_base_state() {
389389
return stream_base_state_;
390390
}
391391

392-
inline uint32_t Environment::get_next_module_id() {
393-
return module_id_counter_++;
394-
}
395-
inline uint32_t Environment::get_next_script_id() {
396-
return script_id_counter_++;
397-
}
398-
inline uint32_t Environment::get_next_function_id() {
399-
return function_id_counter_++;
400-
}
401-
402392
ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope(
403393
Environment* env)
404394
: env_(env) {

‎src/env.h

-12
Original file line numberDiff line numberDiff line change
@@ -723,14 +723,6 @@ class Environment : public MemoryRetainer {
723723
std::vector<std::string> builtins_in_snapshot;
724724

725725
std::unordered_multimap<int, loader::ModuleWrap*> hash_to_module_map;
726-
std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
727-
std::unordered_map<uint32_t, contextify::ContextifyScript*>
728-
id_to_script_map;
729-
std::unordered_map<uint32_t, contextify::CompiledFnEntry*> id_to_function_map;
730-
731-
inline uint32_t get_next_module_id();
732-
inline uint32_t get_next_script_id();
733-
inline uint32_t get_next_function_id();
734726

735727
EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; }
736728

@@ -1054,10 +1046,6 @@ class Environment : public MemoryRetainer {
10541046
uint32_t heap_snapshot_near_heap_limit_ = 0;
10551047
bool heapsnapshot_near_heap_limit_callback_added_ = false;
10561048

1057-
uint32_t module_id_counter_ = 0;
1058-
uint32_t script_id_counter_ = 0;
1059-
uint32_t function_id_counter_ = 0;
1060-
10611049
AliasedInt32Array exit_info_;
10621050

10631051
AliasedUint32Array should_abort_on_uncaught_toggle_;

‎src/env_properties.h

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
V(messaging_transfer_symbol, "messaging_transfer_symbol") \
3737
V(messaging_clone_symbol, "messaging_clone_symbol") \
3838
V(messaging_transfer_list_symbol, "messaging_transfer_list_symbol") \
39+
V(module_meta_symbol, "module_meta_symbol") \
3940
V(oninit_symbol, "oninit") \
4041
V(owner_symbol, "owner_symbol") \
4142
V(onpskexchange_symbol, "onpskexchange") \

‎src/module_wrap.cc

+36-62
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,20 @@ ModuleWrap::ModuleWrap(Environment* env,
5555
Local<Object> object,
5656
Local<Module> module,
5757
Local<String> url)
58-
: BaseObject(env, object),
59-
module_(env->isolate(), module),
60-
id_(env->get_next_module_id()) {
61-
env->id_to_module_map.emplace(id_, this);
62-
58+
: BaseObject(env, object),
59+
module_(env->isolate(), module),
60+
identity_hash_(module->GetIdentityHash()) {
6361
Local<Value> undefined = Undefined(env->isolate());
6462
object->SetInternalField(kURLSlot, url);
6563
object->SetInternalField(kSyntheticEvaluationStepsSlot, undefined);
6664
object->SetInternalField(kContextObjectSlot, undefined);
65+
66+
module_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
6767
}
6868

6969
ModuleWrap::~ModuleWrap() {
7070
HandleScope scope(env()->isolate());
71-
Local<Module> module = module_.Get(env()->isolate());
72-
env()->id_to_module_map.erase(id_);
73-
auto range = env()->hash_to_module_map.equal_range(module->GetIdentityHash());
71+
auto range = env()->hash_to_module_map.equal_range(identity_hash_);
7472
for (auto it = range.first; it != range.second; ++it) {
7573
if (it->second == this) {
7674
env()->hash_to_module_map.erase(it);
@@ -79,6 +77,12 @@ ModuleWrap::~ModuleWrap() {
7977
}
8078
}
8179

80+
// static
81+
void ModuleWrap::WeakCallback(const v8::WeakCallbackInfo<ModuleWrap>& data) {
82+
ModuleWrap* wrap = data.GetParameter();
83+
delete wrap;
84+
}
85+
8286
Local<Context> ModuleWrap::context() const {
8387
Local<Value> obj = object()->GetInternalField(kContextObjectSlot);
8488
if (obj.IsEmpty()) return {};
@@ -96,14 +100,6 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env,
96100
return nullptr;
97101
}
98102

99-
ModuleWrap* ModuleWrap::GetFromID(Environment* env, uint32_t id) {
100-
auto module_wrap_it = env->id_to_module_map.find(id);
101-
if (module_wrap_it == env->id_to_module_map.end()) {
102-
return nullptr;
103-
}
104-
return module_wrap_it->second;
105-
}
106-
107103
// new ModuleWrap(url, context, source, lineOffset, columnOffset)
108104
// new ModuleWrap(url, context, exportNames, syntheticExecutionFunction)
109105
void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
@@ -146,11 +142,6 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
146142
column_offset = args[4].As<Int32>()->Value();
147143
}
148144

149-
Local<PrimitiveArray> host_defined_options =
150-
PrimitiveArray::New(isolate, HostDefinedOptions::kLength);
151-
host_defined_options->Set(isolate, HostDefinedOptions::kType,
152-
Number::New(isolate, ScriptType::kModule));
153-
154145
ShouldNotAbortOnUncaughtScope no_abort_scope(env);
155146
TryCatchScope try_catch(env);
156147

@@ -186,17 +177,16 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
186177
}
187178

188179
Local<String> source_text = args[2].As<String>();
189-
ScriptOrigin origin(isolate,
190-
url,
180+
ScriptOrigin origin(url,
191181
line_offset,
192182
column_offset,
193-
true, // is cross origin
194-
-1, // script id
195-
Local<Value>(), // source map URL
196-
false, // is opaque (?)
197-
false, // is WASM
198-
true, // is ES Module
199-
host_defined_options);
183+
true, // is cross origin
184+
-1, // script id
185+
Local<Value>(), // source map URL
186+
false, // is opaque (?)
187+
false, // is WASM
188+
true, // is ES Module
189+
that); // host defined options
200190
ScriptCompiler::Source source(source_text, origin, cached_data);
201191
ScriptCompiler::CompileOptions options;
202192
if (source.GetCachedData() == nullptr) {
@@ -245,9 +235,8 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
245235

246236
env->hash_to_module_map.emplace(module->GetIdentityHash(), obj);
247237

248-
host_defined_options->Set(isolate, HostDefinedOptions::kID,
249-
Number::New(isolate, obj->id()));
250-
238+
that->Set(context, env->module_meta_symbol(), Object::New(isolate))
239+
.FromJust();
251240
that->SetIntegrityLevel(context, IntegrityLevel::kFrozen);
252241
args.GetReturnValue().Set(that);
253242
}
@@ -568,42 +557,21 @@ static MaybeLocal<Promise> ImportModuleDynamically(
568557
Local<Function> import_callback =
569558
env->host_import_module_dynamically_callback();
570559

571-
Local<FixedArray> options = host_defined_options.As<FixedArray>();
572-
if (options->Length() != HostDefinedOptions::kLength) {
560+
Local<Value> object;
561+
if (host_defined_options->IsValue()) {
562+
object = host_defined_options.As<Value>();
563+
} else {
573564
Local<Promise::Resolver> resolver;
574565
if (!Promise::Resolver::New(context).ToLocal(&resolver)) return {};
575566
resolver
576-
->Reject(context,
577-
v8::Exception::TypeError(FIXED_ONE_BYTE_STRING(
578-
context->GetIsolate(), "Invalid host defined options")))
567+
->Reject(
568+
context,
569+
ERR_INVALID_SCRIPT_CONTEXT(
570+
isolate, "Invalid script context initiating dynamic import"))
579571
.ToChecked();
580572
return handle_scope.Escape(resolver->GetPromise());
581573
}
582574

583-
Local<Value> object;
584-
585-
int type = options->Get(context, HostDefinedOptions::kType)
586-
.As<Number>()
587-
->Int32Value(context)
588-
.ToChecked();
589-
uint32_t id = options->Get(context, HostDefinedOptions::kID)
590-
.As<Number>()
591-
->Uint32Value(context)
592-
.ToChecked();
593-
if (type == ScriptType::kScript) {
594-
contextify::ContextifyScript* wrap = env->id_to_script_map.find(id)->second;
595-
object = wrap->object();
596-
} else if (type == ScriptType::kModule) {
597-
ModuleWrap* wrap = ModuleWrap::GetFromID(env, id);
598-
object = wrap->object();
599-
} else if (type == ScriptType::kFunction) {
600-
auto it = env->id_to_function_map.find(id);
601-
CHECK_NE(it, env->id_to_function_map.end());
602-
object = it->second->object();
603-
} else {
604-
UNREACHABLE();
605-
}
606-
607575
Local<Object> assertions =
608576
createImportAssertionContainer(env, isolate, import_assertions);
609577

@@ -809,6 +777,12 @@ void ModuleWrap::Initialize(Local<Object> target,
809777
V(kEvaluated);
810778
V(kErrored);
811779
#undef V
780+
781+
target
782+
->Set(context,
783+
FIXED_ONE_BYTE_STRING(env->isolate(), "moduleMetaSym"),
784+
env->module_meta_symbol())
785+
.FromJust();
812786
}
813787

814788
} // namespace loader

‎src/module_wrap.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,7 @@ class ModuleWrap : public BaseObject {
5353
tracker->TrackField("resolve_cache", resolve_cache_);
5454
}
5555

56-
inline uint32_t id() { return id_; }
5756
v8::Local<v8::Context> context() const;
58-
static ModuleWrap* GetFromID(node::Environment*, uint32_t id);
5957

6058
SET_MEMORY_INFO_NAME(ModuleWrap)
6159
SET_SELF_SIZE(ModuleWrap)
@@ -100,12 +98,14 @@ class ModuleWrap : public BaseObject {
10098
v8::Local<v8::Module> referrer);
10199
static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
102100

101+
static void WeakCallback(const v8::WeakCallbackInfo<ModuleWrap>& data);
102+
103103
v8::Global<v8::Module> module_;
104+
int identity_hash_;
104105
std::unordered_map<std::string, v8::Global<v8::Promise>> resolve_cache_;
105106
contextify::ContextifyContext* contextify_context_ = nullptr;
106107
bool synthetic_ = false;
107108
bool linked_ = false;
108-
uint32_t id_;
109109
};
110110

111111
} // namespace loader

‎src/node_builtins.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompileInternal(
280280
std::string filename_s = std::string("node:") + id;
281281
Local<String> filename =
282282
OneByteString(isolate, filename_s.c_str(), filename_s.size());
283-
ScriptOrigin origin(isolate, filename, 0, 0, true);
283+
ScriptOrigin origin(filename, 0, 0, true);
284284

285285
ScriptCompiler::CachedData* cached_data = nullptr;
286286
{

0 commit comments

Comments
 (0)