Skip to content

Commit a8faeb0

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 a8faeb0

File tree

9 files changed

+203
-50
lines changed

9 files changed

+203
-50
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: 80 additions & 24 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,12 @@ using v8::UnboundModuleScript;
6362
using v8::Undefined;
6463
using v8::Value;
6564

65+
void ModuleRequest::MemoryInfo(MemoryTracker* tracker) const {
66+
tracker->TrackField("specifier", specifier);
67+
tracker->TrackField("import_attributes", import_attributes);
68+
tracker->TrackField("phase", static_cast<int>(phase));
69+
}
70+
6671
ModuleWrap::ModuleWrap(Realm* realm,
6772
Local<Object> object,
6873
Local<Module> module,
@@ -455,15 +460,36 @@ static Local<Object> createImportAttributesContainer(
455460
return attributes;
456461
}
457462

463+
static std::vector<std::pair<std::string, std::string>>
464+
createImportAttributesPairs(Local<Context> context,
465+
Isolate* isolate,
466+
Local<FixedArray> raw_attributes,
467+
const int elements_per_attribute) {
468+
CHECK_EQ(raw_attributes->Length() % elements_per_attribute, 0);
469+
size_t num_attributes = raw_attributes->Length() / elements_per_attribute;
470+
std::vector<std::pair<std::string, std::string>> attributes;
471+
attributes.reserve(num_attributes);
472+
473+
for (int i = 0; i < raw_attributes->Length(); i += elements_per_attribute) {
474+
Utf8Value key_utf8(isolate, raw_attributes->Get(context, i).As<String>());
475+
Utf8Value value_utf8(isolate,
476+
raw_attributes->Get(context, i + 1).As<String>());
477+
478+
attributes.emplace_back(key_utf8.ToString(), value_utf8.ToString());
479+
}
480+
481+
return attributes;
482+
}
483+
458484
static Local<Array> createModuleRequestsContainer(
459485
Realm* realm, Isolate* isolate, Local<FixedArray> raw_requests) {
460486
EscapableHandleScope scope(isolate);
461487
Local<Context> context = realm->context();
462488
LocalVector<Value> requests(isolate, raw_requests->Length());
463489

464490
for (int i = 0; i < raw_requests->Length(); i++) {
465-
Local<ModuleRequest> module_request =
466-
raw_requests->Get(realm->context(), i).As<ModuleRequest>();
491+
Local<v8::ModuleRequest> module_request =
492+
raw_requests->Get(realm->context(), i).As<v8::ModuleRequest>();
467493

468494
Local<String> specifier = module_request->GetSpecifier();
469495

@@ -496,6 +522,32 @@ static Local<Array> createModuleRequestsContainer(
496522
return scope.Escape(Array::New(isolate, requests.data(), requests.size()));
497523
}
498524

525+
static std::vector<ModuleRequest> V8ModuleRequestsToPrimitive(
526+
Local<Context> context, Local<FixedArray> v8_requests) {
527+
Isolate* isolate = context->GetIsolate();
528+
std::vector<ModuleRequest> requests;
529+
requests.reserve(v8_requests->Length());
530+
531+
for (int i = 0; i < v8_requests->Length(); i++) {
532+
Local<v8::ModuleRequest> v8_module_request =
533+
v8_requests->Get(context, i).As<v8::ModuleRequest>();
534+
535+
Utf8Value specifier_utf8(isolate, v8_module_request->GetSpecifier());
536+
Local<FixedArray> v8_attributes = v8_module_request->GetImportAttributes();
537+
538+
auto import_attributes =
539+
createImportAttributesPairs(context, isolate, v8_attributes, 3);
540+
auto phase = to_phase_constant(v8_module_request->GetPhase());
541+
requests.emplace_back(ModuleRequest{
542+
specifier_utf8.ToString(),
543+
import_attributes,
544+
phase,
545+
});
546+
}
547+
548+
return requests;
549+
}
550+
499551
void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
500552
Realm* realm = Realm::GetCurrent(args);
501553
Isolate* isolate = args.GetIsolate();
@@ -509,7 +561,7 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
509561
realm, isolate, module->GetModuleRequests()));
510562
}
511563

512-
// moduleWrap.link(specifiers, moduleWraps)
564+
// moduleWrap.link(moduleWraps)
513565
void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
514566
Realm* realm = Realm::GetCurrent(args);
515567
Isolate* isolate = args.GetIsolate();
@@ -518,33 +570,29 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
518570
ModuleWrap* dependent;
519571
ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This());
520572

521-
CHECK_EQ(args.Length(), 2);
573+
CHECK_EQ(args.Length(), 1);
522574

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

527-
std::vector<Global<Value>> specifiers_buffer;
528-
if (FromV8Array(context, specifiers, &specifiers_buffer).IsNothing()) {
529-
return;
530-
}
531580
std::vector<Global<Value>> modules_buffer;
532581
if (FromV8Array(context, modules, &modules_buffer).IsNothing()) {
533582
return;
534583
}
535584

536-
for (uint32_t i = 0; i < specifiers->Length(); i++) {
537-
Local<String> specifier_str =
538-
specifiers_buffer[i].Get(isolate).As<String>();
585+
std::vector<ModuleRequest> module_requests =
586+
V8ModuleRequestsToPrimitive(context, requests);
587+
588+
for (uint32_t i = 0; i < module_requests.size(); i++) {
539589
Local<Object> module_object = modules_buffer[i].Get(isolate).As<Object>();
540590

541591
CHECK(
542592
realm->isolate_data()->module_wrap_constructor_template()->HasInstance(
543593
module_object));
544594

545-
Utf8Value specifier(isolate, specifier_str);
546-
dependent->resolve_cache_[specifier.ToString()].Reset(isolate,
547-
module_object);
595+
dependent->resolve_cache_[module_requests[i]].Reset(isolate, module_object);
548596
}
549597
}
550598

@@ -934,14 +982,18 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
934982
return MaybeLocal<Module>();
935983
}
936984

937-
if (dependent->resolve_cache_.count(specifier_std) != 1) {
985+
ModuleRequest request{
986+
specifier_std,
987+
createImportAttributesPairs(context, isolate, import_attributes, 3),
988+
kEvaluationPhase // ResolveModuleCallback for kEvaluationPhase only
989+
};
990+
if (dependent->resolve_cache_.count(request) != 1) {
938991
THROW_ERR_VM_MODULE_LINK_FAILURE(
939992
env, "request for '%s' is not in cache", specifier_std);
940993
return MaybeLocal<Module>();
941994
}
942995

943-
Local<Object> module_object =
944-
dependent->resolve_cache_[specifier_std].Get(isolate);
996+
Local<Object> module_object = dependent->resolve_cache_[request].Get(isolate);
945997
if (module_object.IsEmpty() || !module_object->IsObject()) {
946998
THROW_ERR_VM_MODULE_LINK_FAILURE(
947999
env, "request for '%s' did not return an object", specifier_std);
@@ -975,14 +1027,18 @@ MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
9751027
return MaybeLocal<Object>();
9761028
}
9771029

978-
if (dependent->resolve_cache_.count(specifier_std) != 1) {
1030+
ModuleRequest request{
1031+
specifier_std,
1032+
createImportAttributesPairs(context, isolate, import_attributes, 3),
1033+
kSourcePhase // ResolveSourceCallback for kSourcePhase only
1034+
};
1035+
if (dependent->resolve_cache_.count(request) != 1) {
9791036
THROW_ERR_VM_MODULE_LINK_FAILURE(
9801037
env, "request for '%s' is not in cache", specifier_std);
9811038
return MaybeLocal<Object>();
9821039
}
9831040

984-
Local<Object> module_object =
985-
dependent->resolve_cache_[specifier_std].Get(isolate);
1041+
Local<Object> module_object = dependent->resolve_cache_[request].Get(isolate);
9861042
if (module_object.IsEmpty() || !module_object->IsObject()) {
9871043
THROW_ERR_VM_MODULE_LINK_FAILURE(
9881044
env, "request for '%s' did not return an object", specifier_std);

‎src/module_wrap.h‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,44 @@ enum ModulePhase : int {
3838
kEvaluationPhase = 2,
3939
};
4040

41+
struct ModuleRequest : public MemoryRetainer {
42+
std::string specifier;
43+
std::vector<std::pair<std::string, std::string>> import_attributes;
44+
ModulePhase phase;
45+
46+
ModuleRequest(
47+
std::string specifier,
48+
std::vector<std::pair<std::string, std::string>> import_attributes,
49+
ModulePhase phase)
50+
: specifier(specifier),
51+
import_attributes(import_attributes),
52+
phase(phase) {}
53+
54+
SET_MEMORY_INFO_NAME(ModuleRequest)
55+
SET_SELF_SIZE(ModuleRequest)
56+
void MemoryInfo(MemoryTracker* tracker) const override;
57+
58+
struct Hash {
59+
std::size_t operator()(const ModuleRequest& request) const {
60+
std::size_t h1 = std::hash<std::string>{}(request.specifier);
61+
std::size_t h2 = 0;
62+
for (const auto& attr : request.import_attributes) {
63+
h2 ^= std::hash<std::string>{}(attr.first) ^
64+
std::hash<std::string>{}(attr.second);
65+
}
66+
std::size_t h3 = std::hash<int>{}(request.phase);
67+
// Combine the hashes using a simple XOR and bit shift to reduce
68+
// collisions.
69+
return h1 ^ (h2 << 1) ^ (h3 << 2);
70+
}
71+
};
72+
73+
bool operator==(const ModuleRequest& other) const {
74+
return specifier == other.specifier &&
75+
import_attributes == other.import_attributes && phase == other.phase;
76+
}
77+
};
78+
4179
class ModuleWrap : public BaseObject {
4280
public:
4381
enum InternalFields {
@@ -149,7 +187,8 @@ class ModuleWrap : public BaseObject {
149187
static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
150188

151189
v8::Global<v8::Module> module_;
152-
std::unordered_map<std::string, v8::Global<v8::Object>> resolve_cache_;
190+
std::unordered_map<ModuleRequest, v8::Global<v8::Object>, ModuleRequest::Hash>
191+
resolve_cache_;
153192
contextify::ContextifyContext* contextify_context_ = nullptr;
154193
bool synthetic_ = false;
155194
int module_hash_;

0 commit comments

Comments
 (0)