Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commita86a2e1

Browse files
joyeecheungrichardlau
authored andcommitted
module: use symbol in WeakMap to manage host defined options
Previously when managing the importModuleDynamically callback ofvm.compileFunction(), we use an ID number as the host defined optionand maintain a per-Environment ID -> CompiledFnEntry map to retainthe top-level referrer function returned by vm.compileFunction() inorder to pass it back to the callback, but it would leak because withhow we used v8::Persistent to maintain this reference, V8 would notbe able to understand the cycle and would just think that theCompiledFnEntry was supposed to live forever. We made an attemptto make that reference known to V8 by making the CompiledFnEntry weakand using a private symbol to make CompiledFnEntry stronglyreferences the top-level referrer function in#46785, but that turned out to beunsound, because the there's no guarantee that the top-level functionmust be alive while import() can still be initiated from thatfunction, since V8 could discard the top-level function and only keepinner functions alive, so relying on the top-level function to keepthe CompiledFnEntry alive could result in use-after-free which causeda revert of that fix.With this patch we use a symbol in the host defined options instead ofa number, because with the stage-3 symbol-as-weakmap-keys proposalwe could directly use that symbol to keep the referrer alive using aWeakMap. As a bonus this also keeps the other kinds of referrersalive as long as import() can still be initiated from thatScript/Module, so this also fixes the long-standing crash caused byvm.Script being GC'ed too early when its importModuleDynamicallycallback still needs it.PR-URL:#48510Backport-PR-URL:#51004Refs:#44211Refs:#42080Refs:#47096Refs:#43205Refs:#38695Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent2c2892b commita86a2e1

16 files changed

+185
-191
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,9 @@ import.meta.done();
6969
if(imports.length){
7070
reflect.imports={__proto__:null};
7171
}
72-
const{ setCallbackForWrap}=require('internal/modules/esm/utils');
73-
setCallbackForWrap(m,{
72+
const{ registerModule}=require('internal/modules/esm/utils');
73+
registerModule(m,{
74+
__proto__:null,
7475
initializeImportMeta:(meta,wrap)=>{
7576
meta.exports=reflect.exports;
7677
if(reflect.imports){

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,10 @@ class ModuleLoader {
210210
){
211211
constevalInstance=(url)=>{
212212
const{ ModuleWrap}=internalBinding('module_wrap');
213-
const{setCallbackForWrap}=require('internal/modules/esm/utils');
213+
const{registerModule}=require('internal/modules/esm/utils');
214214
constmodule=newModuleWrap(url,undefined,source,0,0);
215-
setCallbackForWrap(module,{
215+
registerModule(module,{
216+
__proto__:null,
216217
initializeImportMeta:(meta,wrap)=>this.importMetaInitialize(meta,{ url}),
217218
importModuleDynamically:(specifier,{ url},importAttributes)=>{
218219
returnthis.import(specifier,url,importAttributes);

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,9 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
150150
maybeCacheSourceMap(url,source);
151151
debug(`Translating StandardModule${url}`);
152152
constmodule=newModuleWrap(url,undefined,source,0,0);
153-
const{ setCallbackForWrap}=require('internal/modules/esm/utils');
154-
setCallbackForWrap(module,{
153+
const{ registerModule}=require('internal/modules/esm/utils');
154+
registerModule(module,{
155+
__proto__:null,
155156
initializeImportMeta:(meta,wrap)=>this.importMetaInitialize(meta,{ url}),
156157
importModuleDynamically,
157158
});

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

Lines changed: 69 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ const {
77
ObjectFreeze,
88
}=primordials;
99

10+
const{
11+
privateSymbols:{
12+
host_defined_option_symbol,
13+
},
14+
}=internalBinding('util');
1015
const{
1116
ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING,
1217
ERR_INVALID_ARG_VALUE,
@@ -21,16 +26,8 @@ const {
2126
setImportModuleDynamicallyCallback,
2227
setInitializeImportMetaObjectCallback,
2328
}=internalBinding('module_wrap');
24-
const{
25-
getModuleFromWrap,
26-
}=require('internal/vm/module');
2729
constassert=require('internal/assert');
2830

29-
constcallbackMap=newSafeWeakMap();
30-
functionsetCallbackForWrap(wrap,data){
31-
callbackMap.set(wrap,data);
32-
}
33-
3431
letdefaultConditions;
3532
/**
3633
* Returns the default conditions for ES module loading.
@@ -83,34 +80,86 @@ function getConditionsSet(conditions) {
8380
returngetDefaultConditionsSet();
8481
}
8582

83+
/**
84+
*@callback ImportModuleDynamicallyCallback
85+
*@param {string} specifier
86+
*@param {ModuleWrap|ContextifyScript|Function|vm.Module} callbackReferrer
87+
*@param {object} attributes
88+
*@returns { Promise<void>}
89+
*/
90+
91+
/**
92+
*@callback InitializeImportMetaCallback
93+
*@param {object} meta
94+
*@param {ModuleWrap|ContextifyScript|Function|vm.Module} callbackReferrer
95+
*/
96+
97+
/**
98+
*@typedef {{
99+
* callbackReferrer: ModuleWrap|ContextifyScript|Function|vm.Module
100+
* initializeImportMeta? : InitializeImportMetaCallback,
101+
* importModuleDynamically? : ImportModuleDynamicallyCallback
102+
* }} ModuleRegistry
103+
*/
104+
105+
/**
106+
*@type {WeakMap<symbol, ModuleRegistry>}
107+
*/
108+
constmoduleRegistries=newSafeWeakMap();
109+
110+
/**
111+
* V8 would make sure that as long as import() can still be initiated from
112+
* the referrer, the symbol referenced by |host_defined_option_symbol| should
113+
* be alive, which in term would keep the settings object alive through the
114+
* WeakMap, and in turn that keeps the referrer object alive, which would be
115+
* passed into the callbacks.
116+
* The reference goes like this:
117+
* [v8::internal::Script] (via host defined options) ----1--> [idSymbol]
118+
* [callbackReferrer] (via host_defined_option_symbol) ------2------^ |
119+
* ^----------3---- (via WeakMap)------
120+
* 1+3 makes sure that as long as import() can still be initiated, the
121+
* referrer wrap is still around and can be passed into the callbacks.
122+
* 2 is only there so that we can get the id symbol to configure the
123+
* weak map.
124+
*@param {ModuleWrap|ContextifyScript|Function} referrer The referrer to
125+
* get the id symbol from. This is different from callbackReferrer which
126+
* could be set by the caller.
127+
*@param {ModuleRegistry} registry
128+
*/
129+
functionregisterModule(referrer,registry){
130+
constidSymbol=referrer[host_defined_option_symbol];
131+
// To prevent it from being GC'ed.
132+
registry.callbackReferrer??=referrer;
133+
moduleRegistries.set(idSymbol,registry);
134+
}
135+
86136
/**
87137
* Defines the `import.meta` object for a given module.
88-
*@param {object} wrap - Reference to the module.
138+
*@param {symbol} symbol - Reference to the module.
89139
*@param {Record<string, string | Function>} meta - The import.meta object to initialize.
90140
*/
91-
functioninitializeImportMetaObject(wrap,meta){
92-
if(callbackMap.has(wrap)){
93-
const{ initializeImportMeta}=callbackMap.get(wrap);
141+
functioninitializeImportMetaObject(symbol,meta){
142+
if(moduleRegistries.has(symbol)){
143+
const{ initializeImportMeta, callbackReferrer}=moduleRegistries.get(symbol);
94144
if(initializeImportMeta!==undefined){
95-
meta=initializeImportMeta(meta,getModuleFromWrap(wrap)||wrap);
145+
meta=initializeImportMeta(meta,callbackReferrer);
96146
}
97147
}
98148
}
99149

100150
/**
101151
* Asynchronously imports a module dynamically using a callback function. The native callback.
102-
*@param {object} wrap - Reference to the module.
152+
*@param {symbol} symbol - Reference to the module.
103153
*@param {string} specifier - The module specifier string.
104154
*@param {Record<string, string>} attributes - The import attributes object.
105155
*@returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} - The imported module object.
106156
*@throws {ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING} - If the callback function is missing.
107157
*/
108-
asyncfunctionimportModuleDynamicallyCallback(wrap,specifier,attributes){
109-
if(callbackMap.has(wrap)){
110-
const{ importModuleDynamically}=callbackMap.get(wrap);
158+
asyncfunctionimportModuleDynamicallyCallback(symbol,specifier,attributes){
159+
if(moduleRegistries.has(symbol)){
160+
const{ importModuleDynamically, callbackReferrer}=moduleRegistries.get(symbol);
111161
if(importModuleDynamically!==undefined){
112-
returnimportModuleDynamically(
113-
specifier,getModuleFromWrap(wrap)||wrap,attributes);
162+
returnimportModuleDynamically(specifier,callbackReferrer,attributes);
114163
}
115164
}
116165
thrownewERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
@@ -176,7 +225,7 @@ async function initializeHooks() {
176225
}
177226

178227
module.exports={
179-
setCallbackForWrap,
228+
registerModule,
180229
initializeESM,
181230
initializeHooks,
182231
getDefaultConditions,

‎lib/internal/vm.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,10 @@ function internalCompileFunction(code, params, options) {
100100
const{ importModuleDynamicallyWrap}=require('internal/vm/module');
101101
constwrapped=importModuleDynamicallyWrap(importModuleDynamically);
102102
constfunc=result.function;
103-
const{ setCallbackForWrap}=require('internal/modules/esm/utils');
104-
setCallbackForWrap(result.cacheKey,{
105-
importModuleDynamically:(s,_k,i)=>wrapped(s,func,i),
103+
const{ registerModule}=require('internal/modules/esm/utils');
104+
registerModule(func,{
105+
__proto__:null,
106+
importModuleDynamically:wrapped,
106107
});
107108
}
108109

‎lib/internal/vm/module.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ const {
1212
ObjectSetPrototypeOf,
1313
ReflectApply,
1414
SafePromiseAllReturnVoid,
15-
SafeWeakMap,
1615
Symbol,
1716
SymbolToStringTag,
1817
TypeError,
@@ -70,7 +69,6 @@ const STATUS_MAP = {
7069

7170
letglobalModuleId=0;
7271
constdefaultModuleName='vm:module';
73-
constwrapToModuleMap=newSafeWeakMap();
7472

7573
constkWrap=Symbol('kWrap');
7674
constkContext=Symbol('kContext');
@@ -121,25 +119,30 @@ class Module {
121119
});
122120
}
123121

122+
letregistry={__proto__:null};
124123
if(sourceText!==undefined){
125124
this[kWrap]=newModuleWrap(identifier,context,sourceText,
126125
options.lineOffset,options.columnOffset,
127126
options.cachedData);
128-
const{ setCallbackForWrap}=require('internal/modules/esm/utils');
129-
setCallbackForWrap(this[kWrap],{
127+
registry={
128+
__proto__:null,
130129
initializeImportMeta:options.initializeImportMeta,
131130
importModuleDynamically:options.importModuleDynamically ?
132131
importModuleDynamicallyWrap(options.importModuleDynamically) :
133132
undefined,
134-
});
133+
};
135134
}else{
136135
assert(syntheticEvaluationSteps);
137136
this[kWrap]=newModuleWrap(identifier,context,
138137
syntheticExportNames,
139138
syntheticEvaluationSteps);
140139
}
141140

142-
wrapToModuleMap.set(this[kWrap],this);
141+
// This will take precedence over the referrer as the object being
142+
// passed into the callbacks.
143+
registry.callbackReferrer=this;
144+
const{ registerModule}=require('internal/modules/esm/utils');
145+
registerModule(this[kWrap],registry);
143146

144147
this[kContext]=context;
145148
}
@@ -446,5 +449,4 @@ module.exports = {
446449
SourceTextModule,
447450
SyntheticModule,
448451
importModuleDynamicallyWrap,
449-
getModuleFromWrap:(wrap)=>wrapToModuleMap.get(wrap),
450452
};

‎lib/vm.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,9 @@ class Script extends ContextifyScript {
106106
validateFunction(importModuleDynamically,
107107
'options.importModuleDynamically');
108108
const{ importModuleDynamicallyWrap}=require('internal/vm/module');
109-
const{ setCallbackForWrap}=require('internal/modules/esm/utils');
110-
setCallbackForWrap(this,{
109+
const{ registerModule}=require('internal/modules/esm/utils');
110+
registerModule(this,{
111+
__proto__:null,
111112
importModuleDynamically:
112113
importModuleDynamicallyWrap(importModuleDynamically),
113114
});

‎src/env-inl.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -345,16 +345,6 @@ inline AliasedInt32Array& Environment::stream_base_state() {
345345
return stream_base_state_;
346346
}
347347

348-
inlineuint32_tEnvironment::get_next_module_id() {
349-
return module_id_counter_++;
350-
}
351-
inlineuint32_tEnvironment::get_next_script_id() {
352-
return script_id_counter_++;
353-
}
354-
inlineuint32_tEnvironment::get_next_function_id() {
355-
return function_id_counter_++;
356-
}
357-
358348
ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope(
359349
Environment* env)
360350
: env_(env) {

‎src/env.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -698,14 +698,6 @@ class Environment : public MemoryRetainer {
698698
builtins::BuiltinLoader*builtin_loader();
699699

700700
std::unordered_multimap<int, loader::ModuleWrap*> hash_to_module_map;
701-
std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
702-
std::unordered_map<uint32_t, contextify::ContextifyScript*>
703-
id_to_script_map;
704-
std::unordered_map<uint32_t, contextify::CompiledFnEntry*> id_to_function_map;
705-
706-
inlineuint32_tget_next_module_id();
707-
inlineuint32_tget_next_script_id();
708-
inlineuint32_tget_next_function_id();
709701

710702
EnabledDebugList*enabled_debug_list() {return &enabled_debug_list_; }
711703

‎src/env_properties.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
V(arrow_message_private_symbol, "node:arrowMessage") \
2222
V(contextify_context_private_symbol, "node:contextify:context") \
2323
V(decorated_private_symbol, "node:decorated") \
24+
V(host_defined_option_symbol, "node:host_defined_option_symbol") \
2425
V(napi_type_tag, "node:napi:type_tag") \
2526
V(napi_wrapper, "node:napi:wrapper") \
2627
V(untransferable_object_private_symbol, "node:untransferableObject") \
@@ -337,7 +338,6 @@
337338
V(blocklist_constructor_template, v8::FunctionTemplate) \
338339
V(contextify_global_template, v8::ObjectTemplate) \
339340
V(contextify_wrapper_template, v8::ObjectTemplate) \
340-
V(compiled_fn_entry_template, v8::ObjectTemplate) \
341341
V(crypto_key_object_handle_constructor, v8::FunctionTemplate) \
342342
V(env_proxy_template, v8::ObjectTemplate) \
343343
V(env_proxy_ctor_template, v8::FunctionTemplate) \

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp