diff --git a/src/node_ffi.cc b/src/node_ffi.cc index 22aeb8fd22f108..34f999b015123b 100644 --- a/src/node_ffi.cc +++ b/src/node_ffi.cc @@ -20,7 +20,6 @@ using v8::Boolean; using v8::Context; using v8::DontDelete; using v8::DontEnum; -using v8::External; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -39,11 +38,13 @@ using v8::ReadOnly; using v8::String; using v8::TryCatch; using v8::Value; -using v8::WeakCallbackInfo; -using v8::WeakCallbackType; namespace ffi { +void FFIFunctionInfo::MemoryInfo(MemoryTracker* tracker) const { + tracker->TrackField("sb_backing", sb_backing); +} + DynamicLibrary::DynamicLibrary(Environment* env, Local object) : BaseObject(env, object), lib_{}, handle_(nullptr), symbols_() { MakeWeak(); @@ -189,13 +190,44 @@ Maybe DynamicLibrary::PrepareFunction( return Just(PreparedFunction{fn, should_cache_symbol, should_cache_function}); } -void DynamicLibrary::CleanupFunctionInfo( - const WeakCallbackInfo& data) { - FFIFunctionInfo* info = data.GetParameter(); - info->fn.reset(); - info->self.Reset(); - info->library.Reset(); - delete info; +FFIFunctionInfo::FFIFunctionInfo(Environment* env, + Local object, + std::shared_ptr fn, + DynamicLibrary* library) + : BaseObject(env, object), fn(std::move(fn)) { + // Keep the DynamicLibrary instance alive as long as any of its functions are + // alive + object->SetInternalField(FFIFunctionInfo::kLibrary, library->object()); +} + +Local FFIFunctionInfo::GetConstructorTemplate( + IsolateData* isolate_data) { + Local tmpl = + isolate_data->ffi_function_constructor_template(); + if (tmpl.IsEmpty()) { + Isolate* isolate = isolate_data->isolate(); + tmpl = MakeLazilyInitializedJSTemplate(isolate_data, kInternalFieldCount); + Local classname = FIXED_ONE_BYTE_STRING(isolate, "FFIFunctionInfo"); + tmpl->SetClassName(classname); + auto instance = tmpl->InstanceTemplate(); + instance->SetInternalFieldCount(FFIFunctionInfo::kInternalFieldCount); + isolate_data->set_ffi_function_constructor_template(tmpl); + } + return tmpl; +} + +BaseObjectPtr FFIFunctionInfo::Create( + Environment* env, + std::shared_ptr fn, + DynamicLibrary* library) { + Local obj; + if (!GetConstructorTemplate(env->isolate_data()) + ->InstanceTemplate() + ->NewInstance(env->context()) + .ToLocal(&obj)) { + return nullptr; + } + return MakeWeakBaseObject(env, obj, std::move(fn), library); } MaybeLocal DynamicLibrary::CreateFunction( @@ -205,24 +237,18 @@ MaybeLocal DynamicLibrary::CreateFunction( Isolate* isolate = env->isolate(); Local context = env->context(); - // The info is held in a unique_ptr so early-return paths free it. Ownership - // moves to the weak callback via `release()` once `SetWeak` succeeds. - auto info = std::make_unique(); - info->fn = fn; + auto info = FFIFunctionInfo::Create(env, fn, this); DCHECK_EQ(fn->args.size(), fn->arg_type_names.size()); bool use_sb = IsSBEligibleSignature(*fn); bool has_ptr_args = use_sb && SignatureHasPointerArgs(*fn); - Local data = - External::New(isolate, info.get(), v8::kExternalPointerTypeTagDefault); - MaybeLocal maybe_ret = Function::New(context, use_sb ? DynamicLibrary::InvokeFunctionSB : DynamicLibrary::InvokeFunction, - data); + info->object()); Local ret; if (!maybe_ret.ToLocal(&ret)) { return MaybeLocal(); @@ -270,7 +296,8 @@ MaybeLocal DynamicLibrary::CreateFunction( // (strings, Buffers, ArrayBuffers, and ArrayBufferViews). if (has_ptr_args) { Local slow_fn; - if (!Function::New(context, DynamicLibrary::InvokeFunction, data) + if (!Function::New( + context, DynamicLibrary::InvokeFunction, info->object()) .ToLocal(&slow_fn)) { return MaybeLocal(); } @@ -313,12 +340,6 @@ MaybeLocal DynamicLibrary::CreateFunction( } } - info->self.Reset(isolate, ret); - info->library.Reset(isolate, object()); - info->self.SetWeak(info.release(), - DynamicLibrary::CleanupFunctionInfo, - WeakCallbackType::kParameter); - return ret; } @@ -375,8 +396,8 @@ void DynamicLibrary::Close(const FunctionCallbackInfo& args) { void DynamicLibrary::InvokeFunction(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - FFIFunctionInfo* info = static_cast( - args.Data().As()->Value(v8::kExternalPointerTypeTagDefault)); + FFIFunctionInfo* info = Unwrap(args.Data()); + CHECK_NOT_NULL(info); FFIFunction* fn = info->fn.get(); if (fn == nullptr || fn->closed || fn->ptr == nullptr) { @@ -444,8 +465,8 @@ void DynamicLibrary::InvokeFunction(const FunctionCallbackInfo& args) { void DynamicLibrary::InvokeFunctionSB(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - FFIFunctionInfo* info = - static_cast(args.Data().As()->Value()); + FFIFunctionInfo* info = Unwrap(args.Data()); + CHECK_NOT_NULL(info); FFIFunction* fn = info->fn.get(); if (fn == nullptr || fn->closed || fn->ptr == nullptr) { @@ -874,22 +895,22 @@ void DynamicLibrary::RegisterCallback(const FunctionCallbackInfo& args) { return; } - auto callback = new FFICallback{.owner = lib, - .env = env, - .thread_id = std::this_thread::get_id(), - .fn = Global(isolate, fn), - .closure = nullptr, - .ptr = nullptr, - .cif = {}, - .args = std::move(callback_args), - .return_type = return_type}; + auto callback = std::unique_ptr( + new FFICallback{.owner = lib, + .env = env, + .thread_id = std::this_thread::get_id(), + .fn = Global(isolate, fn), + .closure = nullptr, + .ptr = nullptr, + .cif = {}, + .args = std::move(callback_args), + .return_type = return_type}); callback->closure = static_cast( ffi_closure_alloc(sizeof(ffi_closure), &callback->ptr)); if (callback->closure == nullptr) { THROW_ERR_FFI_CALL_FAILED(env, "ffi_closure_alloc failed"); - delete callback; return; } @@ -914,14 +935,13 @@ void DynamicLibrary::RegisterCallback(const FunctionCallbackInfo& args) { } THROW_ERR_FFI_CALL_FAILED(env, msg); - delete callback; return; } status = ffi_prep_closure_loc(callback->closure, &callback->cif, DynamicLibrary::InvokeCallback, - callback, + callback.get(), callback->ptr); if (status != FFI_OK) { const char* msg = "ffi_prep_closure_loc failed"; @@ -938,14 +958,12 @@ void DynamicLibrary::RegisterCallback(const FunctionCallbackInfo& args) { } THROW_ERR_FFI_CALL_FAILED(env, msg); - delete callback; return; } - lib->callbacks_.emplace(callback->ptr, callback); - args.GetReturnValue().Set(BigInt::NewFromUnsigned( - isolate, - static_cast(reinterpret_cast(callback->ptr)))); + auto ret = static_cast(reinterpret_cast(callback->ptr)); + lib->callbacks_.emplace(callback->ptr, std::move(callback)); + args.GetReturnValue().Set(BigInt::NewFromUnsigned(isolate, ret)); } void DynamicLibrary::UnregisterCallback( diff --git a/src/node_ffi.h b/src/node_ffi.h index 22d78a2ad4f35a..1626fd5fa0d756 100644 --- a/src/node_ffi.h +++ b/src/node_ffi.h @@ -29,12 +29,33 @@ struct FFIFunction { std::string return_type_name; }; -struct FFIFunctionInfo { +class FFIFunctionInfo final : public BaseObject { + public: + enum InternalFields { + kLibrary = BaseObject::kInternalFieldCount, + kInternalFieldCount + }; + + FFIFunctionInfo(Environment* env, + v8::Local object, + std::shared_ptr fn, + DynamicLibrary* library); + + void MemoryInfo(MemoryTracker* tracker) const override; + SET_MEMORY_INFO_NAME(FFIFunctionInfo) + SET_SELF_SIZE(FFIFunctionInfo) + + static BaseObjectPtr Create(Environment* env, + std::shared_ptr fn, + DynamicLibrary* library); + static v8::Local GetConstructorTemplate( + IsolateData* isolate_data); + + private: std::shared_ptr fn; - v8::Global self; std::shared_ptr sb_backing; - // Keep the owning DynamicLibrary alive while the generated function is alive. - v8::Global library; + + friend class DynamicLibrary; }; struct FFICallback {