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

Appearance settings

Commit 53eb7ab

Browse filesBrowse files
semimikohaduh95
authored andcommitted
ffi: prevent premature GC of DynamicLibrary
Signed-off-by: semimikoh <ejffjeosms@gmail.com> PR-URL: #63024 Fixes: #63010 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com>
1 parent 58dc92f commit 53eb7ab
Copy full SHA for 53eb7ab

3 files changed

+27-1Lines changed: 27 additions & 1 deletion

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/node_ffi.cc‎

Copy file name to clipboardExpand all lines: src/node_ffi.cc
+2Lines 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);
Collapse file

‎src/node_ffi.h‎

Copy file name to clipboardExpand all lines: src/node_ffi.h
+2Lines 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 {
Collapse file

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

Copy file name to clipboardExpand all lines: test/ffi/test-ffi-dynamic-library.js
+23-1Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.