From 4fc6a8c5ec8a9a720330946af9d1103015c62942 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Wed, 26 Apr 2023 14:00:17 +0200 Subject: Store each FFI::Function in it's own instance variabe in the module to be attached This allows to freeze the FFI::Function immediately, so that it is shareable by Ractor without the need to freeze the module explicit. To make it shareable the typedef hash used for variadic functions is duplicated and made frozen. This creates a small compatibility issue: Only typedefs defined above the variadic function can be used by that function. If a typedef is created after the definition of the variadic function, then this typedef can no longer be used as parameter to that variadic function. Also fix the retrieval of simple (non-struct) global variables per #attached_variables. Closes #975 --- ext/ffi_c/Function.c | 12 +++++------- ext/ffi_c/Variadic.c | 6 +++++- ext/ffi_c/compat.h | 3 +++ lib/ffi/library.rb | 34 +++++++++++++++++----------------- lib/ffi/variadic.rb | 5 ++--- spec/ffi/library_spec.rb | 18 +++++++++++++++++- spec/ffi/struct_by_ref_spec.rb | 1 + spec/ffi/variadic_spec.rb | 2 -- 8 files changed, 50 insertions(+), 31 deletions(-) diff --git a/ext/ffi_c/Function.c b/ext/ffi_c/Function.c index 40d613e..9da6b37 100644 --- a/ext/ffi_c/Function.c +++ b/ext/ffi_c/Function.c @@ -492,6 +492,7 @@ static VALUE function_attach(VALUE self, VALUE module, VALUE name) { Function* fn; + char var[1024]; StringValue(name); TypedData_Get_Struct(self, Function, &function_data_type, fn); @@ -511,14 +512,11 @@ function_attach(VALUE self, VALUE module, VALUE name) } /* - * Stash the Function in a module variable so it does not get garbage collected + * Stash the Function in a module variable so it does not get garbage collected and can be inspected by attached_functions */ - VALUE funcs = rb_iv_get(module, "@ffi_functions"); - if (RB_NIL_P(funcs)) { - funcs = rb_hash_new(); - rb_iv_set(module, "@ffi_functions", funcs); - } - rb_hash_aset(funcs, name, self); + snprintf(var, sizeof(var), "@ffi_function_%s", StringValueCStr(name)); + rb_ractor_make_shareable(self); + rb_iv_set(module, var, self); rb_define_singleton_method(module, StringValueCStr(name), rbffi_MethodHandle_CodeAddress(fn->methodHandle), -1); diff --git a/ext/ffi_c/Variadic.c b/ext/ffi_c/Variadic.c index fb37194..cb5081b 100644 --- a/ext/ffi_c/Variadic.c +++ b/ext/ffi_c/Variadic.c @@ -36,6 +36,9 @@ #include #include #include +#if HAVE_RB_EXT_RACTOR_SAFE +#include +#endif #include #include "rbffi.h" @@ -181,7 +184,8 @@ variadic_initialize(VALUE self, VALUE rbFunction, VALUE rbParameterTypes, VALUE * @fixed and @type_map are used by the parameter mangling ruby code */ rb_iv_set(self, "@fixed", fixed); - rb_iv_set(self, "@type_map", rb_hash_aref(options, ID2SYM(rb_intern("type_map")))); + rb_iv_set(self, "@type_map", rb_obj_dup(rb_hash_aref(options, ID2SYM(rb_intern("type_map"))))); + rb_ractor_make_shareable(self); return retval; } diff --git a/ext/ffi_c/compat.h b/ext/ffi_c/compat.h index a8fdcbe..27a99be 100644 --- a/ext/ffi_c/compat.h +++ b/ext/ffi_c/compat.h @@ -98,5 +98,8 @@ #define FFI_RUBY_TYPED_FROZEN_SHAREABLE RUBY_TYPED_FROZEN_SHAREABLE #endif +#ifndef HAVE_RB_EXT_RACTOR_SAFE +#define rb_ractor_make_shareable(self) rb_obj_freeze(self); +#endif #endif /* RBFFI_COMPAT_H */ diff --git a/lib/ffi/library.rb b/lib/ffi/library.rb index 5656b2d..e681b3e 100644 --- a/lib/ffi/library.rb +++ b/lib/ffi/library.rb @@ -209,7 +209,7 @@ module FFI end raise LoadError unless function - invokers << if arg_types.length > 0 && arg_types[arg_types.length - 1] == FFI::NativeType::VARARGS + invokers << if arg_types[-1] == FFI::NativeType::VARARGS VariadicInvoker.new(function, arg_types, find_type(ret_type), options) else @@ -295,10 +295,9 @@ module FFI # If it is a global struct, just attach directly to the pointer s = s = type.new(address) # Assigning twice to suppress unused variable warning self.module_eval <<-code, __FILE__, __LINE__ - @ffi_gvars = {} unless defined?(@ffi_gvars) - @ffi_gvars[#{mname.inspect}] = s + @ffi_gsvar_#{mname} = s def self.#{mname} - @ffi_gvars[#{mname.inspect}] + @ffi_gsvar_#{mname} end code @@ -310,13 +309,12 @@ module FFI # Attach to this module as mname/mname= # self.module_eval <<-code, __FILE__, __LINE__ - @ffi_gvars = {} unless defined?(@ffi_gvars) - @ffi_gvars[#{mname.inspect}] = s + @ffi_gvar_#{mname} = s def self.#{mname} - @ffi_gvars[#{mname.inspect}][:gvar] + @ffi_gvar_#{mname}[:gvar] end def self.#{mname}=(value) - @ffi_gvars[#{mname.inspect}][:gvar] = value + @ffi_gvar_#{mname}[:gvar] = value end code @@ -543,18 +541,20 @@ module FFI end def attached_functions - @ffi_functions || {} - end - - def attached_variables - (@ffi_gvars || {}).map do |name, gvar| - [name, gvar.class] + instance_variables.grep(/\A@ffi_function_(.*)/) do |m| + [$1, instance_variable_get(m)] end.to_h end - def freeze - super - FFI.make_shareable(@ffi_functions) + def attached_variables + ( + instance_variables.grep(/\A@ffi_gsvar_(.*)/) do |m| + [$1, instance_variable_get(m).class] + end + + instance_variables.grep(/\A@ffi_gvar_(.*)/) do |m| + [$1, instance_variable_get(m).layout[:gvar].type] + end + ).to_h end end end diff --git a/lib/ffi/variadic.rb b/lib/ffi/variadic.rb index 42c5549..800d121 100644 --- a/lib/ffi/variadic.rb +++ b/lib/ffi/variadic.rb @@ -55,11 +55,10 @@ module FFI params = "*args" call = "call" mod.module_eval <<-code - @ffi_functions = {} unless defined?(@ffi_functions) - @ffi_functions[#{mname.inspect}] = invoker + @ffi_function_#{mname} = invoker def self.#{mname}(#{params}) - @ffi_functions[#{mname.inspect}].#{call}(#{params}) + @ffi_function_#{mname}.#{call}(#{params}) end define_method(#{mname.inspect}, &method(:#{mname})) diff --git a/spec/ffi/library_spec.rb b/spec/ffi/library_spec.rb index 97790e5..c9542b9 100644 --- a/spec/ffi/library_spec.rb +++ b/spec/ffi/library_spec.rb @@ -333,9 +333,25 @@ describe "Library" do lib.gvar[:data] = i val = GlobalStruct.new(lib.get) expect(val[:data]).to eq(i) + end + end - expect(lib.attached_variables).to eq({ gvar: GlobalStruct }) + it "can reveal its attached global struct based variables" do + lib = Module.new do |m| + m.extend FFI::Library + ffi_lib TestLibrary::PATH + attach_variable :gvari, "gvar_gstruct", GlobalStruct + end + expect(lib.attached_variables).to eq({ "gvari" => GlobalStruct }) + end + + it "can reveal its attached global variables" do + lib = Module.new do |m| + m.extend FFI::Library + ffi_lib TestLibrary::PATH + attach_variable :gvaro, "gvar_u32", :uint32 end + expect(lib.attached_variables).to eq({ "gvaro" => FFI::Type::UINT32 }) end it "should have shareable constants for Ractor", :ractor do diff --git a/spec/ffi/struct_by_ref_spec.rb b/spec/ffi/struct_by_ref_spec.rb index 0f48fbb..0c01137 100644 --- a/spec/ffi/struct_by_ref_spec.rb +++ b/spec/ffi/struct_by_ref_spec.rb @@ -42,6 +42,7 @@ describe FFI::Struct, ' by_ref' do it "can reveal the mapped type converter" do param_type = @api.attached_functions["struct_test"].param_types[0] + expect(param_type).to be_a(FFI::Type::Mapped) expect(param_type.converter).to be_a(FFI::StructByReference) end end diff --git a/spec/ffi/variadic_spec.rb b/spec/ffi/variadic_spec.rb index fdaf31c..283878b 100644 --- a/spec/ffi/variadic_spec.rb +++ b/spec/ffi/variadic_spec.rb @@ -23,8 +23,6 @@ describe "Function with variadic arguments" do attach_function :testBlockingClose, [ :pointer ], :void attach_function :testCallbackVrDva, :testClosureVrDva, [ :double, :varargs ], :double attach_function :testCallbackVrILva, :testClosureVrILva, [ :int, :long, :varargs ], :long - - freeze end it "takes enum arguments" do -- cgit v1.2.1