Skip to content

Commit c0f1ff2

Browse files
targospull[bot]
authored andcommitted
Revert "vm: fix leak in vm.compileFunction when importModuleDynamically is used"
This reverts commit 986498b. Fixes: nodejs#47096 PR-URL: nodejs#47101 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Danielle Adams <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
1 parent 277f7c6 commit c0f1ff2

File tree

4 files changed

+15
-20
lines changed

4 files changed

+15
-20
lines changed

src/env_properties.h

-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) \
2121
V(arrow_message_private_symbol, "node:arrowMessage") \
2222
V(contextify_context_private_symbol, "node:contextify:context") \
23-
V(compiled_function_entry, "node:compiled_function_entry") \
2423
V(decorated_private_symbol, "node:decorated") \
2524
V(napi_type_tag, "node:napi:type_tag") \
2625
V(napi_wrapper, "node:napi:wrapper") \

src/node_contextify.cc

+12-5
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ using v8::Uint32;
7777
using v8::UnboundScript;
7878
using v8::Value;
7979
using v8::WeakCallbackInfo;
80+
using v8::WeakCallbackType;
8081

8182
// The vm module executes code in a sandboxed environment with a different
8283
// global object than the rest of the code. This is achieved by applying
@@ -1262,7 +1263,8 @@ void ContextifyContext::CompileFunction(
12621263
context).ToLocal(&cache_key)) {
12631264
return;
12641265
}
1265-
new CompiledFnEntry(env, cache_key, id, fn);
1266+
CompiledFnEntry* entry = new CompiledFnEntry(env, cache_key, id, fn);
1267+
env->id_to_function_map.emplace(id, entry);
12661268

12671269
Local<Object> result = Object::New(isolate);
12681270
if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
@@ -1294,18 +1296,23 @@ void ContextifyContext::CompileFunction(
12941296
args.GetReturnValue().Set(result);
12951297
}
12961298

1299+
void CompiledFnEntry::WeakCallback(
1300+
const WeakCallbackInfo<CompiledFnEntry>& data) {
1301+
CompiledFnEntry* entry = data.GetParameter();
1302+
delete entry;
1303+
}
1304+
12971305
CompiledFnEntry::CompiledFnEntry(Environment* env,
12981306
Local<Object> object,
12991307
uint32_t id,
13001308
Local<Function> fn)
1301-
: BaseObject(env, object), id_(id) {
1302-
MakeWeak();
1303-
fn->SetPrivate(env->context(), env->compiled_function_entry(), object);
1304-
env->id_to_function_map.emplace(id, this);
1309+
: BaseObject(env, object), id_(id), fn_(env->isolate(), fn) {
1310+
fn_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
13051311
}
13061312

13071313
CompiledFnEntry::~CompiledFnEntry() {
13081314
env()->id_to_function_map.erase(id_);
1315+
fn_.ClearWeak();
13091316
}
13101317

13111318
static void StartSigintWatchdog(const FunctionCallbackInfo<Value>& args) {

src/node_contextify.h

+3
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,9 @@ class CompiledFnEntry final : public BaseObject {
194194

195195
private:
196196
uint32_t id_;
197+
v8::Global<v8::Function> fn_;
198+
199+
static void WeakCallback(const v8::WeakCallbackInfo<CompiledFnEntry>& data);
197200
};
198201

199202
v8::Maybe<bool> StoreCodeCacheResult(

test/pummel/test-vm-compile-function-leak.js

-14
This file was deleted.

0 commit comments

Comments
 (0)