Skip to content

Commit

Permalink
Revert "vm: fix leak in vm.compileFunction when importModuleDynamical…
Browse files Browse the repository at this point in the history
…ly is used"

This reverts commit 986498b.

Fixes: nodejs#47096
PR-URL: nodejs#47101
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
targos authored and pull[bot] committed Jan 30, 2024
1 parent 277f7c6 commit c0f1ff2
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 20 deletions.
1 change: 0 additions & 1 deletion src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) \
V(arrow_message_private_symbol, "node:arrowMessage") \
V(contextify_context_private_symbol, "node:contextify:context") \
V(compiled_function_entry, "node:compiled_function_entry") \
V(decorated_private_symbol, "node:decorated") \
V(napi_type_tag, "node:napi:type_tag") \
V(napi_wrapper, "node:napi:wrapper") \
Expand Down
17 changes: 12 additions & 5 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ using v8::Uint32;
using v8::UnboundScript;
using v8::Value;
using v8::WeakCallbackInfo;
using v8::WeakCallbackType;

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

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

void CompiledFnEntry::WeakCallback(
const WeakCallbackInfo<CompiledFnEntry>& data) {
CompiledFnEntry* entry = data.GetParameter();
delete entry;
}

CompiledFnEntry::CompiledFnEntry(Environment* env,
Local<Object> object,
uint32_t id,
Local<Function> fn)
: BaseObject(env, object), id_(id) {
MakeWeak();
fn->SetPrivate(env->context(), env->compiled_function_entry(), object);
env->id_to_function_map.emplace(id, this);
: BaseObject(env, object), id_(id), fn_(env->isolate(), fn) {
fn_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
}

CompiledFnEntry::~CompiledFnEntry() {
env()->id_to_function_map.erase(id_);
fn_.ClearWeak();
}

static void StartSigintWatchdog(const FunctionCallbackInfo<Value>& args) {
Expand Down
3 changes: 3 additions & 0 deletions src/node_contextify.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ class CompiledFnEntry final : public BaseObject {

private:
uint32_t id_;
v8::Global<v8::Function> fn_;

static void WeakCallback(const v8::WeakCallbackInfo<CompiledFnEntry>& data);
};

v8::Maybe<bool> StoreCodeCacheResult(
Expand Down
14 changes: 0 additions & 14 deletions test/pummel/test-vm-compile-function-leak.js

This file was deleted.

0 comments on commit c0f1ff2

Please sign in to comment.