From 3015a7aae7ddc9b63149df34b1f12366e07a9563 Mon Sep 17 00:00:00 2001 From: Chris Seaton Date: Tue, 19 May 2020 00:12:47 +0100 Subject: [ruby/fiddle] Improve documentation on how to correctly free memory and free memory in tests (#33) https://github.com/ruby/fiddle/commit/e59cfd708a --- ext/fiddle/lib/fiddle/struct.rb | 11 ++-- ext/fiddle/pointer.c | 26 ++++++++-- test/fiddle/test_c_struct_entry.rb | 12 ++--- test/fiddle/test_c_union_entity.rb | 2 +- test/fiddle/test_import.rb | 103 +++++++++++++++++++++++++------------ test/fiddle/test_pointer.rb | 24 ++++++--- 6 files changed, 125 insertions(+), 53 deletions(-) diff --git a/ext/fiddle/lib/fiddle/struct.rb b/ext/fiddle/lib/fiddle/struct.rb index 8a48a1cb8b..259903d25c 100644 --- a/ext/fiddle/lib/fiddle/struct.rb +++ b/ext/fiddle/lib/fiddle/struct.rb @@ -73,7 +73,12 @@ module Fiddle # # MyStruct = Fiddle::CStructBuilder.create(Fiddle::CUnion, types, members) # - # obj = MyStruct.allocate + # obj = MyStruct.malloc + # begin + # ... + # ensure + # Fiddle.free obj.to_ptr + # end # def create(klass, types, members) new_class = Class.new(klass){ @@ -112,7 +117,7 @@ module Fiddle # Allocates a C struct with the +types+ provided. # - # When the instance is garbage collected, the C function +func+ is called. + # See Fiddle::Pointer.malloc for memory management issues. def CStructEntity.malloc(types, func = nil) addr = Fiddle.malloc(CStructEntity.size(types)) CStructEntity.new(addr, types, func) @@ -267,7 +272,7 @@ module Fiddle # Allocates a C union the +types+ provided. # - # When the instance is garbage collected, the C function +func+ is called. + # See Fiddle::Pointer.malloc for memory management issues. def CUnionEntity.malloc(types, func=nil) addr = Fiddle.malloc(CUnionEntity.size(types)) CUnionEntity.new(addr, types, func) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index 117cc9b826..7c60da477a 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -193,14 +193,34 @@ rb_fiddle_ptr_initialize(int argc, VALUE argv[], VALUE self) /* * call-seq: - * * Fiddle::Pointer.malloc(size, freefunc = nil) => fiddle pointer instance * + * == Examples + * + * # Relying on the garbage collector - may lead to unlimited memory allocated before freeing any, but safe + * pointer = Fiddle::Pointer.malloc(size, Fiddle::RUBY_FREE) + * ... + * + * # Manual freeing + * pointer = Fiddle::Pointer.malloc(size) + * begin + * ... + * ensure + * Fiddle.free pointer + * end + * + * # No free function and no call to free - the native memory will leak if the pointer is garbage collected + * pointer = Fiddle::Pointer.malloc(size) + * ... + * * Allocate +size+ bytes of memory and associate it with an optional * +freefunc+ that will be called when the pointer is garbage collected. - * * +freefunc+ must be an address pointing to a function or an instance of - * Fiddle::Function + * +Fiddle::Function+. Using +freefunc+ may lead to unlimited memory being + * allocated before any is freed as the native memory the pointer references + * does not contribute to triggering the Ruby garbage collector. Consider + * manually freeing the memory as illustrated above. You cannot combine + * the techniques as this may lead to a double-free. */ static VALUE rb_fiddle_ptr_s_malloc(int argc, VALUE argv[], VALUE klass) diff --git a/test/fiddle/test_c_struct_entry.rb b/test/fiddle/test_c_struct_entry.rb index 8ece438f54..33bfee6218 100644 --- a/test/fiddle/test_c_struct_entry.rb +++ b/test/fiddle/test_c_struct_entry.rb @@ -43,7 +43,7 @@ module Fiddle end def test_set_ctypes - union = CStructEntity.malloc [TYPE_INT, TYPE_LONG] + union = CStructEntity.malloc [TYPE_INT, TYPE_LONG], Fiddle::RUBY_FREE union.assign_names %w[int long] # this test is roundabout because the stored ctypes are not accessible @@ -55,20 +55,20 @@ module Fiddle end def test_aref_pointer_array - team = CStructEntity.malloc([[TYPE_VOIDP, 2]]) + team = CStructEntity.malloc([[TYPE_VOIDP, 2]], Fiddle::RUBY_FREE) team.assign_names(["names"]) - alice = Fiddle::Pointer.malloc(6) + alice = Fiddle::Pointer.malloc(6, Fiddle::RUBY_FREE) alice[0, 6] = "Alice\0" - bob = Fiddle::Pointer.malloc(4) + bob = Fiddle::Pointer.malloc(4, Fiddle::RUBY_FREE) bob[0, 4] = "Bob\0" team["names"] = [alice, bob] assert_equal(["Alice", "Bob"], team["names"].map(&:to_s)) end def test_aref_pointer - user = CStructEntity.malloc([TYPE_VOIDP]) + user = CStructEntity.malloc([TYPE_VOIDP], Fiddle::RUBY_FREE) user.assign_names(["name"]) - alice = Fiddle::Pointer.malloc(6) + alice = Fiddle::Pointer.malloc(6, Fiddle::RUBY_FREE) alice[0, 6] = "Alice\0" user["name"] = alice assert_equal("Alice", user["name"].to_s) diff --git a/test/fiddle/test_c_union_entity.rb b/test/fiddle/test_c_union_entity.rb index 5727a20e3b..9310084733 100644 --- a/test/fiddle/test_c_union_entity.rb +++ b/test/fiddle/test_c_union_entity.rb @@ -21,7 +21,7 @@ module Fiddle end def test_set_ctypes - union = CUnionEntity.malloc [TYPE_INT, TYPE_LONG] + union = CUnionEntity.malloc [TYPE_INT, TYPE_LONG], Fiddle::RUBY_FREE union.assign_names %w[int long] # this test is roundabout because the stored ctypes are not accessible diff --git a/test/fiddle/test_import.rb b/test/fiddle/test_import.rb index 867f6960ee..fce8b864d9 100644 --- a/test/fiddle/test_import.rb +++ b/test/fiddle/test_import.rb @@ -57,35 +57,56 @@ module Fiddle def test_struct_memory_access() # check memory operations performed directly on struct my_struct = Fiddle::Importer.struct(['int id']).malloc - my_struct[0, Fiddle::SIZEOF_INT] = "\x01".b * Fiddle::SIZEOF_INT - assert_equal 0x01010101, my_struct.id - - my_struct.id = 0 - assert_equal "\x00".b * Fiddle::SIZEOF_INT, my_struct[0, Fiddle::SIZEOF_INT] + begin + my_struct[0, Fiddle::SIZEOF_INT] = "\x01".b * Fiddle::SIZEOF_INT + assert_equal 0x01010101, my_struct.id + + my_struct.id = 0 + assert_equal "\x00".b * Fiddle::SIZEOF_INT, my_struct[0, Fiddle::SIZEOF_INT] + ensure + Fiddle.free my_struct.to_ptr + end end def test_struct_ptr_array_subscript_multiarg() # check memory operations performed on struct#to_ptr struct = Fiddle::Importer.struct([ 'int x' ]).malloc - ptr = struct.to_ptr + begin + ptr = struct.to_ptr - struct.x = 0x02020202 - assert_equal("\x02".b * Fiddle::SIZEOF_INT, ptr[0, Fiddle::SIZEOF_INT]) + struct.x = 0x02020202 + assert_equal("\x02".b * Fiddle::SIZEOF_INT, ptr[0, Fiddle::SIZEOF_INT]) - ptr[0, Fiddle::SIZEOF_INT] = "\x01".b * Fiddle::SIZEOF_INT - assert_equal 0x01010101, struct.x + ptr[0, Fiddle::SIZEOF_INT] = "\x01".b * Fiddle::SIZEOF_INT + assert_equal 0x01010101, struct.x + ensure + Fiddle.free struct.to_ptr + end end def test_malloc() s1 = LIBC::Timeval.malloc() - s2 = LIBC::Timeval.malloc() - refute_equal(s1.to_ptr.to_i, s2.to_ptr.to_i) + begin + s2 = LIBC::Timeval.malloc() + begin + refute_equal(s1.to_ptr.to_i, s2.to_ptr.to_i) + ensure + Fiddle.free s2.to_ptr + end + ensure + Fiddle.free s1.to_ptr + end end def test_sizeof() assert_equal(SIZEOF_VOIDP, LIBC.sizeof("FILE*")) assert_equal(LIBC::MyStruct.size(), LIBC.sizeof(LIBC::MyStruct)) - assert_equal(LIBC::MyStruct.size(), LIBC.sizeof(LIBC::MyStruct.malloc())) + my_struct = LIBC::MyStruct.malloc() + begin + assert_equal(LIBC::MyStruct.size(), LIBC.sizeof(my_struct)) + ensure + Fiddle.free my_struct.to_ptr + end assert_equal(SIZEOF_LONG_LONG, LIBC.sizeof("long long")) if defined?(SIZEOF_LONG_LONG) end @@ -131,35 +152,51 @@ module Fiddle def test_struct_array_assignment() instance = Fiddle::Importer.struct(["unsigned int stages[3]"]).malloc - instance.stages[0] = 1024 - instance.stages[1] = 10 - instance.stages[2] = 100 - assert_equal 1024, instance.stages[0] - assert_equal 10, instance.stages[1] - assert_equal 100, instance.stages[2] - assert_equal [1024, 10, 100].pack(Fiddle::PackInfo::PACK_MAP[-Fiddle::TYPE_INT] * 3), - instance.to_ptr[0, 3 * Fiddle::SIZEOF_INT] - assert_raise(IndexError) { instance.stages[-1] = 5 } - assert_raise(IndexError) { instance.stages[3] = 5 } + begin + instance.stages[0] = 1024 + instance.stages[1] = 10 + instance.stages[2] = 100 + assert_equal 1024, instance.stages[0] + assert_equal 10, instance.stages[1] + assert_equal 100, instance.stages[2] + assert_equal [1024, 10, 100].pack(Fiddle::PackInfo::PACK_MAP[-Fiddle::TYPE_INT] * 3), + instance.to_ptr[0, 3 * Fiddle::SIZEOF_INT] + assert_raise(IndexError) { instance.stages[-1] = 5 } + assert_raise(IndexError) { instance.stages[3] = 5 } + ensure + Fiddle.free instance.to_ptr + end end def test_struct() s = LIBC::MyStruct.malloc() - s.num = [0,1,2,3,4] - s.c = ?a.ord - s.buff = "012345\377" - assert_equal([0,1,2,3,4], s.num) - assert_equal(?a.ord, s.c) - assert_equal([?0.ord,?1.ord,?2.ord,?3.ord,?4.ord,?5.ord,?\377.ord], s.buff) + begin + s.num = [0,1,2,3,4] + s.c = ?a.ord + s.buff = "012345\377" + assert_equal([0,1,2,3,4], s.num) + assert_equal(?a.ord, s.c) + assert_equal([?0.ord,?1.ord,?2.ord,?3.ord,?4.ord,?5.ord,?\377.ord], s.buff) + ensure + Fiddle.free s.to_ptr + end end def test_gettimeofday() if( defined?(LIBC.gettimeofday) ) timeval = LIBC::Timeval.malloc() - timezone = LIBC::Timezone.malloc() - LIBC.gettimeofday(timeval, timezone) - cur = Time.now() - assert(cur.to_i - 2 <= timeval.tv_sec && timeval.tv_sec <= cur.to_i) + begin + timezone = LIBC::Timezone.malloc() + begin + LIBC.gettimeofday(timeval, timezone) + ensure + Fiddle.free timezone.to_ptr + end + cur = Time.now() + assert(cur.to_i - 2 <= timeval.tv_sec && timeval.tv_sec <= cur.to_i) + ensure + Fiddle.free timeval.to_ptr + end end end diff --git a/test/fiddle/test_pointer.rb b/test/fiddle/test_pointer.rb index 5581c1dea7..c69e4f7142 100644 --- a/test/fiddle/test_pointer.rb +++ b/test/fiddle/test_pointer.rb @@ -84,7 +84,7 @@ module Fiddle end def test_to_ptr_io - buf = Pointer.malloc(10) + buf = Pointer.malloc(10, Fiddle::RUBY_FREE) File.open(__FILE__, 'r') do |f| ptr = Pointer.to_ptr f fread = Function.new(@libc['fread'], @@ -145,7 +145,11 @@ module Fiddle def test_free ptr = Pointer.malloc(4) - assert_nil ptr.free + begin + assert_nil ptr.free + ensure + Fiddle.free ptr + end end def test_free= @@ -173,15 +177,21 @@ module Fiddle def test_size ptr = Pointer.malloc(4) - assert_equal 4, ptr.size - Fiddle.free ptr.to_i + begin + assert_equal 4, ptr.size + ensure + Fiddle.free ptr + end end def test_size= ptr = Pointer.malloc(4) - ptr.size = 10 - assert_equal 10, ptr.size - Fiddle.free ptr.to_i + begin + ptr.size = 10 + assert_equal 10, ptr.size + ensure + Fiddle.free ptr + end end def test_aref_aset -- cgit v1.2.3