Skip to content

Commit 6a8b7e7

Browse files
legendecaslegendecas
committed
module: link module with a module request record
When a module is being statically linked with module requests, if two module requests with a same specifier but different attributes are resolved to two modules, the module requests should be linked to these two modules.
1 parent 4d5ee24 commit 6a8b7e7

File tree

10 files changed

+218
-71
lines changed

10 files changed

+218
-71
lines changed

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,7 @@ class ModuleJob extends ModuleJobBase {
190190
const evaluationDepJobs = Array(moduleRequests.length);
191191
ObjectSetPrototypeOf(evaluationDepJobs, null);
192192

193-
// Specifiers should be aligned with the moduleRequests array in order.
194-
const specifiers = Array(moduleRequests.length);
193+
// Modules should be aligned with the moduleRequests array in order.
195194
const modulePromises = Array(moduleRequests.length);
196195
// Track each loop for whether it is an evaluation phase or source phase request.
197196
let isEvaluation;
@@ -217,11 +216,10 @@ class ModuleJob extends ModuleJobBase {
217216
return job.modulePromise;
218217
});
219218
modulePromises[idx] = modulePromise;
220-
specifiers[idx] = specifier;
221219
}
222220

223221
const modules = await SafePromiseAllReturnArrayLike(modulePromises);
224-
this.module.link(specifiers, modules);
222+
this.module.link(modules);
225223

226224
return evaluationDepJobs;
227225
}
@@ -433,22 +431,20 @@ class ModuleJobSync extends ModuleJobBase {
433431
this.#loader.loadCache.set(this.url, this.type, this);
434432
try {
435433
const moduleRequests = this.module.getModuleRequests();
436-
// Specifiers should be aligned with the moduleRequests array in order.
437-
const specifiers = Array(moduleRequests.length);
434+
// Modules should be aligned with the moduleRequests array in order.
438435
const modules = Array(moduleRequests.length);
439436
const evaluationDepJobs = Array(moduleRequests.length);
440437
let j = 0;
441438
for (let i = 0; i < moduleRequests.length; ++i) {
442439
const { specifier, attributes, phase } = moduleRequests[i];
443440
const job = this.#loader.getModuleJobForRequire(specifier, this.url, attributes, phase);
444-
specifiers[i] = specifier;
445441
modules[i] = job.module;
446442
if (phase === kEvaluationPhase) {
447443
evaluationDepJobs[j++] = job;
448444
}
449445
}
450446
evaluationDepJobs.length = j;
451-
this.module.link(specifiers, modules);
447+
this.module.link(modules);
452448
this.linked = evaluationDepJobs;
453449
} finally {
454450
// Restore it - if it succeeds, we'll reset in the caller; Otherwise it's

‎lib/internal/vm/module.js‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -148,23 +148,10 @@ class Module {
148148
});
149149
}
150150

151-
let registry = { __proto__: null };
152151
if (sourceText !== undefined) {
153152
this[kWrap] = new ModuleWrap(identifier, context, sourceText,
154153
options.lineOffset, options.columnOffset,
155154
options.cachedData);
156-
registry = {
157-
__proto__: null,
158-
initializeImportMeta: options.initializeImportMeta,
159-
importModuleDynamically: options.importModuleDynamically ?
160-
importModuleDynamicallyWrap(options.importModuleDynamically) :
161-
undefined,
162-
};
163-
// This will take precedence over the referrer as the object being
164-
// passed into the callbacks.
165-
registry.callbackReferrer = this;
166-
const { registerModule } = require('internal/modules/esm/utils');
167-
registerModule(this[kWrap], registry);
168155
} else {
169156
assert(syntheticEvaluationSteps);
170157
this[kWrap] = new ModuleWrap(identifier, context,
@@ -315,6 +302,19 @@ class SourceTextModule extends Module {
315302
importModuleDynamically,
316303
});
317304

305+
const registry = {
306+
__proto__: null,
307+
initializeImportMeta: options.initializeImportMeta,
308+
importModuleDynamically: options.importModuleDynamically ?
309+
importModuleDynamicallyWrap(options.importModuleDynamically) :
310+
undefined,
311+
};
312+
// This will take precedence over the referrer as the object being
313+
// passed into the callbacks.
314+
registry.callbackReferrer = this;
315+
const { registerModule } = require('internal/modules/esm/utils');
316+
registerModule(this[kWrap], registry);
317+
318318
this.#moduleRequests = ObjectFreeze(ArrayPrototypeMap(this[kWrap].getModuleRequests(), (request) => {
319319
return ObjectFreeze({
320320
__proto__: null,
@@ -329,8 +329,7 @@ class SourceTextModule extends Module {
329329
this.#statusOverride = 'linking';
330330

331331
// Iterates the module requests and links with the linker.
332-
// Specifiers should be aligned with the moduleRequests array in order.
333-
const specifiers = Array(this.#moduleRequests.length);
332+
// Modules should be aligned with the moduleRequests array in order.
334333
const modulePromises = Array(this.#moduleRequests.length);
335334
// Iterates with index to avoid calling into userspace with `Symbol.iterator`.
336335
for (let idx = 0; idx < this.#moduleRequests.length; idx++) {
@@ -357,12 +356,11 @@ class SourceTextModule extends Module {
357356
return module[kWrap];
358357
});
359358
modulePromises[idx] = modulePromise;
360-
specifiers[idx] = specifier;
361359
}
362360

363361
try {
364362
const modules = await SafePromiseAllReturnArrayLike(modulePromises);
365-
this[kWrap].link(specifiers, modules);
363+
this[kWrap].link(modules);
366364
} catch (e) {
367365
this.#error = e;
368366
throw e;

‎src/module_wrap.cc‎

Lines changed: 90 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ using v8::Message;
4646
using v8::MicrotaskQueue;
4747
using v8::Module;
4848
using v8::ModuleImportPhase;
49-
using v8::ModuleRequest;
5049
using v8::Name;
5150
using v8::Nothing;
5251
using v8::Null;
@@ -63,6 +62,66 @@ using v8::UnboundModuleScript;
6362
using v8::Undefined;
6463
using v8::Value;
6564

65+
ModulePhase to_phase_constant(ModuleImportPhase phase) {
66+
switch (phase) {
67+
case ModuleImportPhase::kEvaluation:
68+
return kEvaluationPhase;
69+
case ModuleImportPhase::kSource:
70+
return kSourcePhase;
71+
}
72+
UNREACHABLE();
73+
}
74+
75+
void ModuleRequest::MemoryInfo(MemoryTracker* tracker) const {
76+
tracker->TrackField("specifier", specifier);
77+
tracker->TrackField("import_attributes", import_attributes);
78+
tracker->TrackField("phase", static_cast<int>(phase));
79+
}
80+
81+
template <int elements_per_attribute>
82+
ModuleRequest ModuleRequest::From(Local<Context> context,
83+
Local<String> specifier,
84+
Local<FixedArray> import_attributes,
85+
ModulePhase phase) {
86+
CHECK_EQ(import_attributes->Length() % elements_per_attribute, 0);
87+
Isolate* isolate = context->GetIsolate();
88+
std::size_t h1 = specifier->GetIdentityHash();
89+
size_t num_attributes = import_attributes->Length() / elements_per_attribute;
90+
std::vector<std::pair<std::string, std::string>> attributes;
91+
attributes.reserve(num_attributes);
92+
93+
std::size_t h2 = 0;
94+
95+
for (int i = 0; i < import_attributes->Length();
96+
i += elements_per_attribute) {
97+
Local<String> v8_key = import_attributes->Get(context, i).As<String>();
98+
Local<String> v8_value =
99+
import_attributes->Get(context, i + 1).As<String>();
100+
Utf8Value key_utf8(isolate, v8_key);
101+
Utf8Value value_utf8(isolate, v8_value);
102+
103+
attributes.emplace_back(key_utf8.ToString(), value_utf8.ToString());
104+
h2 ^= v8_key->GetIdentityHash();
105+
h2 ^= v8_value->GetIdentityHash();
106+
}
107+
108+
std::size_t h3 = std::hash<int>{}(static_cast<int>(phase));
109+
// Combine the hashes using a simple XOR and bit shift to reduce
110+
// collisions.
111+
std::size_t hash = h1 ^ (h2 << 1) ^ (h3 << 2);
112+
113+
Utf8Value utf8_specifier(isolate, specifier);
114+
return ModuleRequest{utf8_specifier.ToString(), attributes, phase, hash};
115+
}
116+
117+
ModuleRequest ModuleRequest::From(Local<Context> context,
118+
Local<v8::ModuleRequest> v8_request) {
119+
return From(context,
120+
v8_request->GetSpecifier(),
121+
v8_request->GetImportAttributes(),
122+
to_phase_constant(v8_request->GetPhase()));
123+
}
124+
66125
ModuleWrap::ModuleWrap(Realm* realm,
67126
Local<Object> object,
68127
Local<Module> module,
@@ -422,16 +481,6 @@ MaybeLocal<Module> ModuleWrap::CompileSourceTextModule(
422481
return scope.Escape(module);
423482
}
424483

425-
ModulePhase to_phase_constant(ModuleImportPhase phase) {
426-
switch (phase) {
427-
case ModuleImportPhase::kEvaluation:
428-
return kEvaluationPhase;
429-
case ModuleImportPhase::kSource:
430-
return kSourcePhase;
431-
}
432-
UNREACHABLE();
433-
}
434-
435484
static Local<Object> createImportAttributesContainer(
436485
Realm* realm,
437486
Isolate* isolate,
@@ -462,8 +511,8 @@ static Local<Array> createModuleRequestsContainer(
462511
LocalVector<Value> requests(isolate, raw_requests->Length());
463512

464513
for (int i = 0; i < raw_requests->Length(); i++) {
465-
Local<ModuleRequest> module_request =
466-
raw_requests->Get(realm->context(), i).As<ModuleRequest>();
514+
Local<v8::ModuleRequest> module_request =
515+
raw_requests->Get(realm->context(), i).As<v8::ModuleRequest>();
467516

468517
Local<String> specifier = module_request->GetSpecifier();
469518

@@ -509,7 +558,7 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
509558
realm, isolate, module->GetModuleRequests()));
510559
}
511560

512-
// moduleWrap.link(specifiers, moduleWraps)
561+
// moduleWrap.link(moduleWraps)
513562
void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
514563
Realm* realm = Realm::GetCurrent(args);
515564
Isolate* isolate = args.GetIsolate();
@@ -518,33 +567,28 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
518567
ModuleWrap* dependent;
519568
ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This());
520569

521-
CHECK_EQ(args.Length(), 2);
570+
CHECK_EQ(args.Length(), 1);
522571

523-
Local<Array> specifiers = args[0].As<Array>();
524-
Local<Array> modules = args[1].As<Array>();
525-
CHECK_EQ(specifiers->Length(), modules->Length());
572+
Local<FixedArray> requests =
573+
dependent->module_.Get(isolate)->GetModuleRequests();
574+
Local<Array> modules = args[0].As<Array>();
575+
CHECK_EQ(modules->Length(), static_cast<uint32_t>(requests->Length()));
526576

527-
std::vector<Global<Value>> specifiers_buffer;
528-
if (FromV8Array(context, specifiers, &specifiers_buffer).IsNothing()) {
529-
return;
530-
}
531577
std::vector<Global<Value>> modules_buffer;
532578
if (FromV8Array(context, modules, &modules_buffer).IsNothing()) {
533579
return;
534580
}
535581

536-
for (uint32_t i = 0; i < specifiers->Length(); i++) {
537-
Local<String> specifier_str =
538-
specifiers_buffer[i].Get(isolate).As<String>();
582+
for (uint32_t i = 0; i < modules_buffer.size(); i++) {
539583
Local<Object> module_object = modules_buffer[i].Get(isolate).As<Object>();
540584

541585
CHECK(
542586
realm->isolate_data()->module_wrap_constructor_template()->HasInstance(
543587
module_object));
544588

545-
Utf8Value specifier(isolate, specifier_str);
546-
dependent->resolve_cache_[specifier.ToString()].Reset(isolate,
547-
module_object);
589+
ModuleRequest module_request = ModuleRequest::From(
590+
context, requests->Get(context, i).As<v8::ModuleRequest>());
591+
dependent->resolve_cache_[module_request].Reset(isolate, module_object);
548592
}
549593
}
550594

@@ -924,27 +968,28 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
924968
return MaybeLocal<Module>();
925969
}
926970

927-
Utf8Value specifier_utf8(isolate, specifier);
928-
std::string specifier_std(*specifier_utf8, specifier_utf8.length());
971+
// ResolveModuleCallback for kEvaluationPhase only.
972+
const ModulePhase phase = ModulePhase::kEvaluationPhase;
973+
ModuleRequest request =
974+
ModuleRequest::From(context, specifier, import_attributes, phase);
929975

930976
ModuleWrap* dependent = GetFromModule(env, referrer);
931977
if (dependent == nullptr) {
932978
THROW_ERR_VM_MODULE_LINK_FAILURE(
933-
env, "request for '%s' is from invalid module", specifier_std);
979+
env, "request for '%s' is from invalid module", request.specifier);
934980
return MaybeLocal<Module>();
935981
}
936982

937-
if (dependent->resolve_cache_.count(specifier_std) != 1) {
983+
if (dependent->resolve_cache_.count(request) != 1) {
938984
THROW_ERR_VM_MODULE_LINK_FAILURE(
939-
env, "request for '%s' is not in cache", specifier_std);
985+
env, "request for '%s' is not in cache", request.specifier);
940986
return MaybeLocal<Module>();
941987
}
942988

943-
Local<Object> module_object =
944-
dependent->resolve_cache_[specifier_std].Get(isolate);
989+
Local<Object> module_object = dependent->resolve_cache_[request].Get(isolate);
945990
if (module_object.IsEmpty() || !module_object->IsObject()) {
946991
THROW_ERR_VM_MODULE_LINK_FAILURE(
947-
env, "request for '%s' did not return an object", specifier_std);
992+
env, "request for '%s' did not return an object", request.specifier);
948993
return MaybeLocal<Module>();
949994
}
950995

@@ -965,27 +1010,28 @@ MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
9651010
return MaybeLocal<Object>();
9661011
}
9671012

968-
Utf8Value specifier_utf8(isolate, specifier);
969-
std::string specifier_std(*specifier_utf8, specifier_utf8.length());
1013+
// ResolveSourceCallback for kSourcePhase only.
1014+
const ModulePhase phase = ModulePhase::kSourcePhase;
1015+
ModuleRequest request =
1016+
ModuleRequest::From(context, specifier, import_attributes, phase);
9701017

9711018
ModuleWrap* dependent = GetFromModule(env, referrer);
9721019
if (dependent == nullptr) {
9731020
THROW_ERR_VM_MODULE_LINK_FAILURE(
974-
env, "request for '%s' is from invalid module", specifier_std);
1021+
env, "request for '%s' is from invalid module", request.specifier);
9751022
return MaybeLocal<Object>();
9761023
}
9771024

978-
if (dependent->resolve_cache_.count(specifier_std) != 1) {
1025+
if (dependent->resolve_cache_.count(request) != 1) {
9791026
THROW_ERR_VM_MODULE_LINK_FAILURE(
980-
env, "request for '%s' is not in cache", specifier_std);
1027+
env, "request for '%s' is not in cache", request.specifier);
9811028
return MaybeLocal<Object>();
9821029
}
9831030

984-
Local<Object> module_object =
985-
dependent->resolve_cache_[specifier_std].Get(isolate);
1031+
Local<Object> module_object = dependent->resolve_cache_[request].Get(isolate);
9861032
if (module_object.IsEmpty() || !module_object->IsObject()) {
9871033
THROW_ERR_VM_MODULE_LINK_FAILURE(
988-
env, "request for '%s' did not return an object", specifier_std);
1034+
env, "request for '%s' did not return an object", request.specifier);
9891035
return MaybeLocal<Object>();
9901036
}
9911037

0 commit comments

Comments
 (0)