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 2004d8d

Browse filesBrowse files
addaleaxaduh95
addaleax
authored and
aduh95
committed
ffi: make FFIFunctionInfo a BaseObject subclass
This brings the typical benefits of using `BaseObject`, such as a reduced risk of memory management pitfalls (e.g. accidental `v8::Global` recursive references) and diagnostic tracking. Signed-off-by: Anna Henningsen <anna@addaleax.net> PR-URL: #63071 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 53eb7ab commit 2004d8d
Copy full SHA for 2004d8d

2 files changed

+76-34Lines changed: 76 additions & 34 deletions

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
+51-30Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ using v8::Boolean;
2020
using v8::Context;
2121
using v8::DontDelete;
2222
using v8::DontEnum;
23-
using v8::External;
2423
using v8::Function;
2524
using v8::FunctionCallbackInfo;
2625
using v8::FunctionTemplate;
@@ -39,11 +38,13 @@ using v8::ReadOnly;
3938
using v8::String;
4039
using v8::TryCatch;
4140
using v8::Value;
42-
using v8::WeakCallbackInfo;
43-
using v8::WeakCallbackType;
4441

4542
namespace ffi {
4643

44+
void FFIFunctionInfo::MemoryInfo(MemoryTracker* tracker) const {
45+
tracker->TrackField("sb_backing", sb_backing);
46+
}
47+
4748
DynamicLibrary::DynamicLibrary(Environment* env, Local<Object> object)
4849
: BaseObject(env, object), lib_{}, handle_(nullptr), symbols_() {
4950
MakeWeak();
@@ -189,13 +190,44 @@ Maybe<DynamicLibrary::PreparedFunction> DynamicLibrary::PrepareFunction(
189190
return Just(PreparedFunction{fn, should_cache_symbol, should_cache_function});
190191
}
191192

192-
void DynamicLibrary::CleanupFunctionInfo(
193-
const WeakCallbackInfo<FFIFunctionInfo>& data) {
194-
FFIFunctionInfo* info = data.GetParameter();
195-
info->fn.reset();
196-
info->self.Reset();
197-
info->library.Reset();
198-
delete info;
193+
FFIFunctionInfo::FFIFunctionInfo(Environment* env,
194+
Local<Object> object,
195+
std::shared_ptr<FFIFunction> fn,
196+
DynamicLibrary* library)
197+
: BaseObject(env, object), fn(std::move(fn)) {
198+
// Keep the DynamicLibrary instance alive as long as any of its functions are
199+
// alive
200+
object->SetInternalField(FFIFunctionInfo::kLibrary, library->object());
201+
}
202+
203+
Local<FunctionTemplate> FFIFunctionInfo::GetConstructorTemplate(
204+
IsolateData* isolate_data) {
205+
Local<FunctionTemplate> tmpl =
206+
isolate_data->ffi_function_constructor_template();
207+
if (tmpl.IsEmpty()) {
208+
Isolate* isolate = isolate_data->isolate();
209+
tmpl = MakeLazilyInitializedJSTemplate(isolate_data, kInternalFieldCount);
210+
Local<String> classname = FIXED_ONE_BYTE_STRING(isolate, "FFIFunctionInfo");
211+
tmpl->SetClassName(classname);
212+
auto instance = tmpl->InstanceTemplate();
213+
instance->SetInternalFieldCount(FFIFunctionInfo::kInternalFieldCount);
214+
isolate_data->set_ffi_function_constructor_template(tmpl);
215+
}
216+
return tmpl;
217+
}
218+
219+
BaseObjectPtr<FFIFunctionInfo> FFIFunctionInfo::Create(
220+
Environment* env,
221+
std::shared_ptr<FFIFunction> fn,
222+
DynamicLibrary* library) {
223+
Local<Object> obj;
224+
if (!GetConstructorTemplate(env->isolate_data())
225+
->InstanceTemplate()
226+
->NewInstance(env->context())
227+
.ToLocal(&obj)) {
228+
return nullptr;
229+
}
230+
return MakeWeakBaseObject<FFIFunctionInfo>(env, obj, std::move(fn), library);
199231
}
200232

201233
MaybeLocal<Function> DynamicLibrary::CreateFunction(
@@ -205,24 +237,18 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
205237
Isolate* isolate = env->isolate();
206238
Local<Context> context = env->context();
207239

208-
// The info is held in a unique_ptr so early-return paths free it. Ownership
209-
// moves to the weak callback via `release()` once `SetWeak` succeeds.
210-
auto info = std::make_unique<FFIFunctionInfo>();
211-
info->fn = fn;
240+
auto info = FFIFunctionInfo::Create(env, fn, this);
212241

213242
DCHECK_EQ(fn->args.size(), fn->arg_type_names.size());
214243

215244
bool use_sb = IsSBEligibleSignature(*fn);
216245
bool has_ptr_args = use_sb && SignatureHasPointerArgs(*fn);
217246

218-
Local<External> data =
219-
External::New(isolate, info.get(), v8::kExternalPointerTypeTagDefault);
220-
221247
MaybeLocal<Function> maybe_ret =
222248
Function::New(context,
223249
use_sb ? DynamicLibrary::InvokeFunctionSB
224250
: DynamicLibrary::InvokeFunction,
225-
data);
251+
info->object());
226252
Local<Function> ret;
227253
if (!maybe_ret.ToLocal(&ret)) {
228254
return MaybeLocal<Function>();
@@ -270,7 +296,8 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
270296
// (strings, Buffers, ArrayBuffers, and ArrayBufferViews).
271297
if (has_ptr_args) {
272298
Local<Function> slow_fn;
273-
if (!Function::New(context, DynamicLibrary::InvokeFunction, data)
299+
if (!Function::New(
300+
context, DynamicLibrary::InvokeFunction, info->object())
274301
.ToLocal(&slow_fn)) {
275302
return MaybeLocal<Function>();
276303
}
@@ -313,12 +340,6 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
313340
}
314341
}
315342

316-
info->self.Reset(isolate, ret);
317-
info->library.Reset(isolate, object());
318-
info->self.SetWeak(info.release(),
319-
DynamicLibrary::CleanupFunctionInfo,
320-
WeakCallbackType::kParameter);
321-
322343
return ret;
323344
}
324345

@@ -375,8 +396,8 @@ void DynamicLibrary::Close(const FunctionCallbackInfo<Value>& args) {
375396

376397
void DynamicLibrary::InvokeFunction(const FunctionCallbackInfo<Value>& args) {
377398
Environment* env = Environment::GetCurrent(args);
378-
FFIFunctionInfo* info = static_cast<FFIFunctionInfo*>(
379-
args.Data().As<External>()->Value(v8::kExternalPointerTypeTagDefault));
399+
FFIFunctionInfo* info = Unwrap<FFIFunctionInfo>(args.Data());
400+
CHECK_NOT_NULL(info);
380401
FFIFunction* fn = info->fn.get();
381402

382403
if (fn == nullptr || fn->closed || fn->ptr == nullptr) {
@@ -444,8 +465,8 @@ void DynamicLibrary::InvokeFunction(const FunctionCallbackInfo<Value>& args) {
444465

445466
void DynamicLibrary::InvokeFunctionSB(const FunctionCallbackInfo<Value>& args) {
446467
Environment* env = Environment::GetCurrent(args);
447-
FFIFunctionInfo* info =
448-
static_cast<FFIFunctionInfo*>(args.Data().As<External>()->Value());
468+
FFIFunctionInfo* info = Unwrap<FFIFunctionInfo>(args.Data());
469+
CHECK_NOT_NULL(info);
449470
FFIFunction* fn = info->fn.get();
450471

451472
if (fn == nullptr || fn->closed || fn->ptr == nullptr) {
@@ -920,7 +941,7 @@ void DynamicLibrary::RegisterCallback(const FunctionCallbackInfo<Value>& args) {
920941
status = ffi_prep_closure_loc(callback->closure,
921942
&callback->cif,
922943
DynamicLibrary::InvokeCallback,
923-
callback,
944+
callback.get(),
924945
callback->ptr);
925946
if (status != FFI_OK) {
926947
const char* msg = "ffi_prep_closure_loc failed";
Collapse file

‎src/node_ffi.h‎

Copy file name to clipboardExpand all lines: src/node_ffi.h
+25-4Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,33 @@ struct FFIFunction {
2929
std::string return_type_name;
3030
};
3131

32-
struct FFIFunctionInfo {
32+
class FFIFunctionInfo final : public BaseObject {
33+
public:
34+
enum InternalFields {
35+
kLibrary = BaseObject::kInternalFieldCount,
36+
kInternalFieldCount
37+
};
38+
39+
FFIFunctionInfo(Environment* env,
40+
v8::Local<v8::Object> object,
41+
std::shared_ptr<FFIFunction> fn,
42+
DynamicLibrary* library);
43+
44+
void MemoryInfo(MemoryTracker* tracker) const override;
45+
SET_MEMORY_INFO_NAME(FFIFunctionInfo)
46+
SET_SELF_SIZE(FFIFunctionInfo)
47+
48+
static BaseObjectPtr<FFIFunctionInfo> Create(Environment* env,
49+
std::shared_ptr<FFIFunction> fn,
50+
DynamicLibrary* library);
51+
static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
52+
IsolateData* isolate_data);
53+
54+
private:
3355
std::shared_ptr<FFIFunction> fn;
34-
v8::Global<v8::Function> self;
3556
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;
57+
58+
friend class DynamicLibrary;
3859
};
3960

4061
struct FFICallback {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.