Skip to content

Commit 70074c6

Browse files
trivikrtrivikr
authored
ffi: make Fast API functions collectable
V8 strongly caches instantiations of low-serial FunctionTemplates, which pinned Fast API functions and, through them, their DynamicLibrary and trampoline. FunctionTemplate::New() has no opt-out, so add an ObjectTemplate property to make these templates uncacheable, then delete it from the new function. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: claude:opus-5.5 PR-URL: #67177 Fixes: #67176 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 96de0e2 commit 70074c6

3 files changed

Lines changed: 80 additions & 2 deletions

File tree

‎src/env_properties.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@
416416
V(fdclose_constructor_template, v8::ObjectTemplate) \
417417
V(ffi_dynamic_library_constructor_template, v8::FunctionTemplate) \
418418
V(ffi_function_constructor_template, v8::FunctionTemplate) \
419+
V(ffi_uncacheable_placeholder_template, v8::ObjectTemplate) \
419420
V(filehandlereadwrap_template, v8::ObjectTemplate) \
420421
V(fips_indicator_event_template, v8::DictionaryTemplate) \
421422
V(free_list_statistics_template, v8::DictionaryTemplate) \

‎src/node_ffi.cc‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ using v8::LocalVector;
3737
using v8::Maybe;
3838
using v8::MaybeLocal;
3939
using v8::Object;
40+
using v8::ObjectTemplate;
4041
using v8::PropertyAttribute;
4142
using v8::ReadOnly;
4243
using v8::Signature;
@@ -277,6 +278,32 @@ BaseObjectPtr<FFIFunctionInfo> FFIFunctionInfo::Create(
277278
return MakeWeakBaseObject<FFIFunctionInfo>(env, obj, std::move(fn), library);
278279
}
279280

281+
// V8 strongly caches instantiations of low-serial FunctionTemplates, which
282+
// would root the function and, through FFIFunctionInfo, its library and
283+
// trampoline. An ObjectTemplate property is the only public way to make a
284+
// FunctionTemplate uncacheable (see v8::Template::Set()), so add one and delete
285+
// it from the instance. It is shared because its own instantiation is cached.
286+
static MaybeLocal<Function> GetUncachedFunction(Environment* env,
287+
Local<FunctionTemplate> tmpl) {
288+
Local<Context> context = env->context();
289+
IsolateData* isolate_data = env->isolate_data();
290+
291+
Local<ObjectTemplate> placeholder =
292+
isolate_data->ffi_uncacheable_placeholder_template();
293+
if (placeholder.IsEmpty()) {
294+
placeholder = ObjectTemplate::New(env->isolate());
295+
isolate_data->set_ffi_uncacheable_placeholder_template(placeholder);
296+
}
297+
tmpl->Set(env->pointer_string(), placeholder);
298+
299+
Local<Function> ret;
300+
if (!tmpl->GetFunction(context).ToLocal(&ret) ||
301+
!ret->Delete(context, env->pointer_string()).FromMaybe(false)) {
302+
return MaybeLocal<Function>();
303+
}
304+
return ret;
305+
}
306+
280307
MaybeLocal<Function> DynamicLibrary::CreateFunction(
281308
Environment* env,
282309
const std::string& name,
@@ -340,7 +367,7 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
340367
v8::ConstructorBehavior::kThrow,
341368
v8::SideEffectType::kHasSideEffect,
342369
&info->fast_metadata->c_function);
343-
maybe_ret = tmpl->GetFunction(context);
370+
maybe_ret = GetUncachedFunction(env, tmpl);
344371
} else {
345372
// Non-Fast signatures either use the SharedBuffer invoker, where JS writes
346373
// argument slots before calling with no arguments, or the generic invoker
@@ -492,7 +519,7 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
492519
v8::SideEffectType::kHasSideEffect,
493520
&info->fast_buffer_metadata->c_function);
494521
Local<Function> fast_buffer_invoke;
495-
if (!tmpl->GetFunction(context).ToLocal(&fast_buffer_invoke)) {
522+
if (!GetUncachedFunction(env, tmpl).ToLocal(&fast_buffer_invoke)) {
496523
return MaybeLocal<Function>();
497524
}
498525
if (!ret->DefineOwnProperty(context,

‎test/ffi/test-ffi-dynamic-library.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,56 @@ test('a dropped function wrapper is collectable', async () => {
260260
}
261261
});
262262

263+
for (const symbol of [
264+
'multiply_f64', // Fast API, no JS wrapper
265+
'add_i32', // Fast API, JS wrapper
266+
'get_counter', // Fast API, no arguments
267+
'pointer_to_usize', // Fast API, JS wrapper and Buffer invoker
268+
]) {
269+
test(`dropping ${symbol} releases the function and its library`, async () => {
270+
let lib = new ffi.DynamicLibrary(libraryPath);
271+
let fn = lib.getFunction(symbol, fixtureSymbols[symbol]);
272+
const fnRef = new WeakRef(fn);
273+
const libRef = new WeakRef(lib);
274+
lib = null;
275+
fn = null;
276+
277+
await gcUntil(`dropping ${symbol} releases the function and its library`, () => {
278+
return fnRef.deref() === undefined && libRef.deref() === undefined;
279+
});
280+
});
281+
}
282+
283+
test('an optimized Fast API call target can be collected', async () => {
284+
const lib = new ffi.DynamicLibrary(libraryPath);
285+
286+
try {
287+
function callMultiply(fn) {
288+
return fn(20, 22);
289+
}
290+
eval('%PrepareFunctionForOptimization(callMultiply)');
291+
292+
// Collect a function that optimized code has called, then make sure a new
293+
// function for the same symbol works through a fresh trampoline.
294+
let fn = lib.getFunction('multiply_f64', fixtureSymbols.multiply_f64);
295+
assert.strictEqual(callMultiply(fn), 440);
296+
assert.strictEqual(callMultiply(fn), 440);
297+
eval('%OptimizeFunctionOnNextCall(callMultiply)');
298+
assert.strictEqual(callMultiply(fn), 440);
299+
300+
const ref = new WeakRef(fn);
301+
fn = null;
302+
await gcUntil('an optimized Fast API call target can be collected', () => {
303+
return ref.deref() === undefined;
304+
});
305+
306+
fn = lib.getFunction('multiply_f64', fixtureSymbols.multiply_f64);
307+
assert.strictEqual(callMultiply(fn), 440);
308+
} finally {
309+
lib.close();
310+
}
311+
});
312+
263313
test('FFI functions keep their owning library alive', async () => {
264314
let lib = new ffi.DynamicLibrary(libraryPath);
265315
const addI32 = lib.getFunction('add_i32', fixtureSymbols.add_i32);

0 commit comments

Comments
 (0)