diff --git a/.gitignore b/.gitignore index ff9d895bb..bb9e3f9a0 100644 --- a/.gitignore +++ b/.gitignore @@ -21,5 +21,6 @@ Gemfile.lock types_log *.gem embed-test.rb.log +gc_compact.rb.log spec/ffi/embed-test/ext/Makefile spec/ffi/embed-test/ext/embed_test.bundle diff --git a/ext/ffi_c/Call.c b/ext/ffi_c/Call.c index 8db60f2e4..f77a71157 100644 --- a/ext/ffi_c/Call.c +++ b/ext/ffi_c/Call.c @@ -91,6 +91,23 @@ static inline void* getPointer(VALUE value, int type); static ID id_to_ptr, id_map_symbol, id_to_native; +static VALUE rbffi_pin_string_content(VALUE str) +{ +#ifdef RSTRING_EMBED_LEN_MAX + if (likely(RB_TYPE_P(str, T_STRING)) && !unlikely(RB_OBJ_FROZEN(str))){ + /* Expand the string capacity so we aren't using embedded string memory + * but heap memory for the string content. + * This is to avoid relocation of the content by GC.compact + */ + long expand_len = RSTRING_EMBED_LEN_MAX + 1 - RSTRING_LEN(str); + if (likely(expand_len > 0)) { + rb_str_modify_expand( str, expand_len ); + } + } +#endif + return str; +} + void rbffi_SetupCallParams(int argc, VALUE* argv, int paramCount, Type** paramTypes, FFIStorage* paramStorage, void** ffiValues, @@ -298,7 +315,9 @@ rbffi_SetupCallParams(int argc, VALUE* argv, int paramCount, Type** paramTypes, param->ptr = NULL; } else { - param->ptr = StringValueCStr(argv[argidx]); + VALUE str = argv[argidx]; + rbffi_pin_string_content(str); + param->ptr = StringValueCStr(str); } ADJ(param, ADDRESS); @@ -435,6 +454,7 @@ getPointer(VALUE value, int type) } else if (type == T_STRING) { + rbffi_pin_string_content(value); return StringValuePtr(value); } else if (type == T_NIL) { diff --git a/spec/ffi/fixtures/StringTest.c b/spec/ffi/fixtures/StringTest.c index ce1f3f2be..e498fb1bd 100644 --- a/spec/ffi/fixtures/StringTest.c +++ b/spec/ffi/fixtures/StringTest.c @@ -32,3 +32,6 @@ string_null(void) return NULL; } +void store_string(char* str, char **ptr) { + *ptr = str; +} diff --git a/spec/ffi/gc_compact.rb b/spec/ffi/gc_compact.rb new file mode 100644 index 000000000..b242fa1fb --- /dev/null +++ b/spec/ffi/gc_compact.rb @@ -0,0 +1,44 @@ +# -*- encoding: utf-8 -*- +# +# This file is part of ruby-ffi. +# For licensing, see LICENSE.SPECS +# + +require File.expand_path(File.join(File.dirname(__FILE__), "spec_helper")) + +# These specs are not run as regular rspec file, but as a separate process from string_spec.rb. +# That's due to GC.verify_compaction_references is very memory consuming since it doubles the needed memory with each call. +# So it's intentionally that the file name miss the "_spec" prefix. + +if GC.respond_to?(:compact) + describe "GC.compact" do + module GcCompactTestLib + extend FFI::Library + ffi_lib TestLibrary::PATH + attach_function :store_string, [ :string, :pointer ], :void + attach_function :store_pointer, :store_string, [ :pointer, :pointer ], :void + end + + A = [] + F = [] + [0, 1, 23, 24].each_with_index do |len, idx| + A[idx] = "a" * len + F[idx] = ("a" * len).freeze + + it "preserves a #{len} character string" do + mem = FFI::MemoryPointer.new :pointer, 3 + GcCompactTestLib.store_string(A[idx], mem) + GcCompactTestLib.store_pointer(A[idx], mem + 1*FFI::TYPE_POINTER.size) + GcCompactTestLib.store_string(F[idx], mem + 2*FFI::TYPE_POINTER.size) + + GC.verify_compaction_references(toward: :empty, double_heap: true) + + expect( mem.get_pointer(0*FFI::TYPE_POINTER.size).read_string ).to eq("a" * len) + expect( mem.get_pointer(1*FFI::TYPE_POINTER.size).read_string ).to eq("a" * len) + expect( mem.get_pointer(2*FFI::TYPE_POINTER.size).read_string ).to eq("a" * len) + expect( A[idx] ).to eq("a" * len) + expect( F[idx] ).to eq("a" * len) + end + end + end +end diff --git a/spec/ffi/string_spec.rb b/spec/ffi/string_spec.rb index 7cd855896..e5b3c408c 100644 --- a/spec/ffi/string_spec.rb +++ b/spec/ffi/string_spec.rb @@ -23,6 +23,11 @@ module StrLibTest expect(StrLibTest.pointer_buffer_equals(str + "a", str + "b", str.bytesize + 1)).to eq(0) end + it "Frozen String can be passed to :pointer and :string argument" do + str = "string buffer".freeze + expect(StrLibTest.pointer_buffer_equals(str, str, str.bytesize)).to eq(1) + end + it "Poison null byte raises error" do s = "123\0abc" expect{ StrLibTest.pointer_buffer_equals("", s, 0) }.to raise_error(ArgumentError) @@ -98,4 +103,12 @@ module StrLibTest ptrary.write_array_of_pointer(ary) expect { ptrary.get_array_of_string(-1) }.to raise_error(IndexError) end + + it "with GC.compact" do + skip "GC.compact is unavailable" unless GC.respond_to?(:compact) + + out = external_run("rspec", "gc_compact.rb") + expect(out).to match(/ 0 failures/) + end + end