Skip to content

Commit

Permalink
module: migrate to script context based host defined options
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
legendecas committed Oct 8, 2022
1 parent f7e3eb8 commit 54f0d90
Show file tree
Hide file tree
Showing 17 changed files with 103 additions and 149 deletions.
6 changes: 3 additions & 3 deletions lib/internal/modules/esm/create_dynamic_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ ${ArrayPrototypeJoin(ArrayPrototypeMap(imports, createImport), '\n')}
${ArrayPrototypeJoin(ArrayPrototypeMap(exports, createExport), '\n')}
import.meta.done();
`;
const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
const { ModuleWrap, moduleMetaSym } = internalBinding('module_wrap');
const m = new ModuleWrap(`${url}`, undefined, source, 0, 0);

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

callbackMap.set(m, {
m[moduleMetaSym] = {
initializeImportMeta: (meta, wrap) => {
meta.exports = reflect.exports;
if (reflect.imports)
Expand All @@ -61,7 +61,7 @@ import.meta.done();
}
};
},
});
};

return {
module: m,
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,13 +362,13 @@ class ESMLoader {
url = pathToFileURL(`${process.cwd()}/[eval${++this.evalIndex}]`).href
) {
const evalInstance = (url) => {
const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
const { ModuleWrap, moduleMetaSym } = internalBinding('module_wrap');
const module = new ModuleWrap(url, undefined, source, 0, 0);
callbackMap.set(module, {
module[moduleMetaSym] = {
importModuleDynamically: (specifier, { url }, importAssertions) => {
return this.import(specifier, url, importAssertions);
}
});
};

return module;
};
Expand Down
6 changes: 2 additions & 4 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,8 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
maybeCacheSourceMap(url, source);
debug(`Translating StandardModule ${url}`);
const module = new ModuleWrap(url, undefined, source, 0, 0);
moduleWrap.callbackMap.set(module, {
initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }),
importModuleDynamically,
});
module[moduleWrap.moduleMetaSym].initializeImportMeta = (meta, wrap) => this.importMetaInitialize(meta, { url });
module[moduleWrap.moduleMetaSym].importModuleDynamically = importModuleDynamically;
return module;
});

Expand Down
12 changes: 6 additions & 6 deletions lib/internal/process/esm_loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ const {
} = require('internal/vm/module');

exports.initializeImportMetaObject = function(wrap, meta) {
const { callbackMap } = internalBinding('module_wrap');
if (callbackMap.has(wrap)) {
const { initializeImportMeta } = callbackMap.get(wrap);
const { moduleMetaSym } = internalBinding('module_wrap');
if (wrap[moduleMetaSym]) {
const { initializeImportMeta } = wrap[moduleMetaSym];
if (initializeImportMeta !== undefined) {
initializeImportMeta(meta, getModuleFromWrap(wrap) || wrap);
}
Expand All @@ -29,9 +29,9 @@ exports.initializeImportMetaObject = function(wrap, meta) {

exports.importModuleDynamicallyCallback =
async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
const { callbackMap } = internalBinding('module_wrap');
if (callbackMap.has(wrap)) {
const { importModuleDynamically } = callbackMap.get(wrap);
const { moduleMetaSym } = internalBinding('module_wrap');
if (wrap[moduleMetaSym]) {
const { importModuleDynamically } = wrap[moduleMetaSym];
if (importModuleDynamically !== undefined) {
return importModuleDynamically(
specifier, getModuleFromWrap(wrap) || wrap, assertions);
Expand Down
3 changes: 0 additions & 3 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -549,9 +549,6 @@ function initializeCJSLoader() {
}

function initializeESMLoader() {
// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
internalBinding('module_wrap').callbackMap = new SafeWeakMap();

if (getEmbedderOptions().shouldNotRegisterESMLoader) return;

const {
Expand Down
8 changes: 3 additions & 5 deletions lib/internal/vm/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,10 @@ class Module {
options.lineOffset, options.columnOffset,
options.cachedData);

binding.callbackMap.set(this[kWrap], {
initializeImportMeta: options.initializeImportMeta,
importModuleDynamically: options.importModuleDynamically ?
this[kWrap][binding.moduleMetaSym].initializeImportMeta = options.initializeImportMeta;
this[kWrap][binding.moduleMetaSym].importModuleDynamically = options.importModuleDynamically ?
importModuleDynamicallyWrap(options.importModuleDynamically) :
undefined,
});
undefined;
} else {
assert(syntheticEvaluationSteps);
this[kWrap] = new ModuleWrap(identifier, context,
Expand Down
12 changes: 6 additions & 6 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ class Script extends ContextifyScript {
'options.importModuleDynamically');
const { importModuleDynamicallyWrap } =
require('internal/vm/module');
const { callbackMap } = internalBinding('module_wrap');
callbackMap.set(this, {
const { moduleMetaSym } = internalBinding('module_wrap');
this[moduleMetaSym] = {
importModuleDynamically:
importModuleDynamicallyWrap(importModuleDynamically),
});
};
}
}

Expand Down Expand Up @@ -382,12 +382,12 @@ function compileFunction(code, params, options = kEmptyObject) {
'options.importModuleDynamically');
const { importModuleDynamicallyWrap } =
require('internal/vm/module');
const { callbackMap } = internalBinding('module_wrap');
const { moduleMetaSym } = internalBinding('module_wrap');
const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
const func = result.function;
callbackMap.set(result.cacheKey, {
result.cacheKey[moduleMetaSym] = {
importModuleDynamically: (s, _k, i) => wrapped(s, func, i),
});
};
}

return result.function;
Expand Down
10 changes: 0 additions & 10 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -383,16 +383,6 @@ inline AliasedInt32Array& Environment::stream_base_state() {
return stream_base_state_;
}

inline uint32_t Environment::get_next_module_id() {
return module_id_counter_++;
}
inline uint32_t Environment::get_next_script_id() {
return script_id_counter_++;
}
inline uint32_t Environment::get_next_function_id() {
return function_id_counter_++;
}

ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope(
Environment* env)
: env_(env) {
Expand Down
12 changes: 0 additions & 12 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -774,14 +774,6 @@ class Environment : public MemoryRetainer {
std::vector<std::string> builtins_in_snapshot;

std::unordered_multimap<int, loader::ModuleWrap*> hash_to_module_map;
std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
std::unordered_map<uint32_t, contextify::ContextifyScript*>
id_to_script_map;
std::unordered_map<uint32_t, contextify::CompiledFnEntry*> id_to_function_map;

inline uint32_t get_next_module_id();
inline uint32_t get_next_script_id();
inline uint32_t get_next_function_id();

EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; }

Expand Down Expand Up @@ -1097,10 +1089,6 @@ class Environment : public MemoryRetainer {
uint32_t heap_snapshot_near_heap_limit_ = 0;
bool heapsnapshot_near_heap_limit_callback_added_ = false;

uint32_t module_id_counter_ = 0;
uint32_t script_id_counter_ = 0;
uint32_t function_id_counter_ = 0;

AliasedUint32Array exiting_;

AliasedUint32Array should_abort_on_uncaught_toggle_;
Expand Down
1 change: 1 addition & 0 deletions src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
V(messaging_transfer_symbol, "messaging_transfer_symbol") \
V(messaging_clone_symbol, "messaging_clone_symbol") \
V(messaging_transfer_list_symbol, "messaging_transfer_list_symbol") \
V(module_meta_symbol, "module_meta_symbol") \
V(oninit_symbol, "oninit") \
V(owner_symbol, "owner_symbol") \
V(onpskexchange_symbol, "onpskexchange") \
Expand Down
61 changes: 11 additions & 50 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ ModuleWrap::ModuleWrap(Environment* env,
Local<Module> module,
Local<String> url)
: BaseObject(env, object),
module_(env->isolate(), module),
id_(env->get_next_module_id()) {
env->id_to_module_map.emplace(id_, this);
module_(env->isolate(), module) {

Local<Value> undefined = Undefined(env->isolate());
object->SetInternalField(kURLSlot, url);
Expand All @@ -69,7 +67,6 @@ ModuleWrap::ModuleWrap(Environment* env,
ModuleWrap::~ModuleWrap() {
HandleScope scope(env()->isolate());
Local<Module> module = module_.Get(env()->isolate());
env()->id_to_module_map.erase(id_);
auto range = env()->hash_to_module_map.equal_range(module->GetIdentityHash());
for (auto it = range.first; it != range.second; ++it) {
if (it->second == this) {
Expand All @@ -96,14 +93,6 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env,
return nullptr;
}

ModuleWrap* ModuleWrap::GetFromID(Environment* env, uint32_t id) {
auto module_wrap_it = env->id_to_module_map.find(id);
if (module_wrap_it == env->id_to_module_map.end()) {
return nullptr;
}
return module_wrap_it->second;
}

// new ModuleWrap(url, context, source, lineOffset, columnOffset)
// new ModuleWrap(url, context, exportNames, syntheticExecutionFunction)
void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -146,11 +135,6 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
column_offset = args[4].As<Int32>()->Value();
}

Local<PrimitiveArray> host_defined_options =
PrimitiveArray::New(isolate, HostDefinedOptions::kLength);
host_defined_options->Set(isolate, HostDefinedOptions::kType,
Number::New(isolate, ScriptType::kModule));

ShouldNotAbortOnUncaughtScope no_abort_scope(env);
TryCatchScope try_catch(env);

Expand Down Expand Up @@ -195,8 +179,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
Local<Value>(), // source map URL
false, // is opaque (?)
false, // is WASM
true, // is ES Module
host_defined_options);
true); // is ES Module
ScriptCompiler::Source source(source_text, origin, cached_data);
ScriptCompiler::CompileOptions options;
if (source.GetCachedData() == nullptr) {
Expand Down Expand Up @@ -245,9 +228,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {

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

host_defined_options->Set(isolate, HostDefinedOptions::kID,
Number::New(isolate, obj->id()));

that->Set(context, env->module_meta_symbol(), Object::New(isolate)).FromJust();
that->SetIntegrityLevel(context, IntegrityLevel::kFrozen);
args.GetReturnValue().Set(that);
}
Expand Down Expand Up @@ -568,42 +549,19 @@ static MaybeLocal<Promise> ImportModuleDynamically(
Local<Function> import_callback =
env->host_import_module_dynamically_callback();

Local<FixedArray> options = host_defined_options.As<FixedArray>();
if (options->Length() != HostDefinedOptions::kLength) {
Local<Value> object;
if (host_defined_options->IsValue()) {
object = host_defined_options.As<Value>();
} else {
Local<Promise::Resolver> resolver;
if (!Promise::Resolver::New(context).ToLocal(&resolver)) return {};
resolver
->Reject(context,
v8::Exception::TypeError(FIXED_ONE_BYTE_STRING(
context->GetIsolate(), "Invalid host defined options")))
ERR_INVALID_SCRIPT_CONTEXT(isolate, "Invalid script context initiating dynamic import"))
.ToChecked();
return handle_scope.Escape(resolver->GetPromise());
}

Local<Value> object;

int type = options->Get(context, HostDefinedOptions::kType)
.As<Number>()
->Int32Value(context)
.ToChecked();
uint32_t id = options->Get(context, HostDefinedOptions::kID)
.As<Number>()
->Uint32Value(context)
.ToChecked();
if (type == ScriptType::kScript) {
contextify::ContextifyScript* wrap = env->id_to_script_map.find(id)->second;
object = wrap->object();
} else if (type == ScriptType::kModule) {
ModuleWrap* wrap = ModuleWrap::GetFromID(env, id);
object = wrap->object();
} else if (type == ScriptType::kFunction) {
auto it = env->id_to_function_map.find(id);
CHECK_NE(it, env->id_to_function_map.end());
object = it->second->object();
} else {
UNREACHABLE();
}

Local<Object> assertions =
createImportAssertionContainer(env, isolate, import_assertions);

Expand Down Expand Up @@ -809,6 +767,9 @@ void ModuleWrap::Initialize(Local<Object> target,
V(kEvaluated);
V(kErrored);
#undef V

target->Set(context, FIXED_ONE_BYTE_STRING(env->isolate(), "moduleMetaSym"),
env->module_meta_symbol()).FromJust();
}

} // namespace loader
Expand Down
3 changes: 0 additions & 3 deletions src/module_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ class ModuleWrap : public BaseObject {
tracker->TrackField("resolve_cache", resolve_cache_);
}

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

SET_MEMORY_INFO_NAME(ModuleWrap)
SET_SELF_SIZE(ModuleWrap)
Expand Down Expand Up @@ -105,7 +103,6 @@ class ModuleWrap : public BaseObject {
contextify::ContextifyContext* contextify_context_ = nullptr;
bool synthetic_ = false;
bool linked_ = false;
uint32_t id_;
};

} // namespace loader
Expand Down
Loading

0 comments on commit 54f0d90

Please sign in to comment.