Skip to content

Commit 5d3ffc4

Browse files
committed
ffi: prevent premature GC of DynamicLibrary
Signed-off-by: semimikoh <[email protected]>
1 parent 8173251 commit 5d3ffc4

3 files changed

Lines changed: 27 additions & 1 deletion

File tree

src/node_ffi.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ void DynamicLibrary::CleanupFunctionInfo(
194194
FFIFunctionInfo* info = data.GetParameter();
195195
info->fn.reset();
196196
info->self.Reset();
197+
info->library.Reset();
197198
delete info;
198199
}
199200

@@ -313,6 +314,7 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
313314
}
314315

315316
info->self.Reset(isolate, ret);
317+
info->library.Reset(isolate, object());
316318
info->self.SetWeak(info.release(),
317319
DynamicLibrary::CleanupFunctionInfo,
318320
WeakCallbackType::kParameter);

src/node_ffi.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ struct FFIFunctionInfo {
3333
std::shared_ptr<FFIFunction> fn;
3434
v8::Global<v8::Function> self;
3535
std::shared_ptr<v8::BackingStore> sb_backing;
36+
// Keep the owning DynamicLibrary alive while the generated function is alive.
37+
v8::Global<v8::Object> library;
3638
};
3739

3840
struct FFICallback {

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
// Flags: --experimental-ffi
1+
// Flags: --experimental-ffi --expose-gc
22
'use strict';
33
const common = require('../common');
44
common.skipIfFFIMissing();
5+
const { gcUntil } = require('../common/gc');
56
const assert = require('node:assert');
67
const { test } = require('node:test');
78
const ffi = require('node:ffi');
@@ -117,6 +118,27 @@ test('getFunction caches signatures consistently', () => {
117118
}
118119
});
119120

121+
test('FFI functions keep their owning library alive', async () => {
122+
let lib = new ffi.DynamicLibrary(libraryPath);
123+
const addI32 = lib.getFunction('add_i32', fixtureSymbols.add_i32);
124+
const ref = new WeakRef(lib);
125+
126+
lib = null;
127+
128+
for (let i = 0; i < 5; i++) {
129+
await gcUntil(
130+
'FFI function keeps its owning library alive',
131+
() => true,
132+
1,
133+
);
134+
assert.ok(ref.deref() instanceof ffi.DynamicLibrary);
135+
assert.strictEqual(addI32(20, 22), 42);
136+
}
137+
138+
ref.deref().close();
139+
assert.throws(() => addI32(20, 22), /Library is closed/);
140+
});
141+
120142
test('closed libraries reject subsequent operations', () => {
121143
const { lib, functions } = ffi.dlopen(libraryPath, {
122144
add_i32: fixtureSymbols.add_i32,

0 commit comments

Comments
 (0)