From 7100f65cff5239f7fc2e090f20fc253c97b87fdf Mon Sep 17 00:00:00 2001 From: Chris Seaton Date: Sun, 31 May 2020 22:16:27 +0100 Subject: [PATCH 01/10] Add a Fiddle::Pointer#free! method --- ext/fiddle/pointer.c | 69 ++++++++++++++++++++++++++++++------- test/fiddle/test_pointer.rb | 69 +++++++++++++++++++++++++++++-------- 2 files changed, 112 insertions(+), 26 deletions(-) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index 7c60da47..b9fb0fab 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -24,6 +24,7 @@ struct ptr_data { void *ptr; long size; freefunc_t free; + int freed; VALUE wrap[2]; }; @@ -57,14 +58,19 @@ fiddle_ptr_mark(void *ptr) } static void -fiddle_ptr_free(void *ptr) +fiddle_ptr_free_ptr(void *ptr) { struct ptr_data *data = ptr; - if (data->ptr) { - if (data->free) { - (*(data->free))(data->ptr); - } + if (data->ptr && data->free && !data->freed) { + (*(data->free))(data->ptr); } + data->freed = 1; +} + +static void +fiddle_ptr_free(void *ptr) +{ + fiddle_ptr_free_ptr(ptr); xfree(ptr); } @@ -89,6 +95,7 @@ rb_fiddle_ptr_new2(VALUE klass, void *ptr, long size, freefunc_t func) val = TypedData_Make_Struct(klass, struct ptr_data, &fiddle_ptr_data_type, data); data->ptr = ptr; data->free = func; + data->freed = 0; data->size = size; return val; @@ -140,6 +147,7 @@ rb_fiddle_ptr_s_allocate(VALUE klass) data->ptr = 0; data->size = 0; data->free = 0; + data->freed = 0; return obj; } @@ -197,11 +205,16 @@ rb_fiddle_ptr_initialize(int argc, VALUE argv[], VALUE self) * * == Examples * + * # Manually freeing but relying on the garbage collector otherwise + * pointer = Fiddle::Pointer.malloc(size, Fiddle::RUBY_FREE) + * ... + * pointer.free! + * * # 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 + * # Only manually freeing * pointer = Fiddle::Pointer.malloc(size) * begin * ... @@ -214,13 +227,15 @@ rb_fiddle_ptr_initialize(int argc, VALUE argv[], VALUE self) * ... * * Allocate +size+ bytes of memory and associate it with an optional - * +freefunc+ that will be called when the pointer is garbage collected. + * +freefunc+ that will be called when the pointer is garbage collected, + * if not already manually called via *free!*. + * * +freefunc+ must be an address pointing to a function or an instance of - * +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. + * +Fiddle::Function+. Using +freefunc+ without using +free!+ 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 + * can combine the techniques as +freefunc+ will not be called twice. */ static VALUE rb_fiddle_ptr_s_malloc(int argc, VALUE argv[], VALUE klass) @@ -370,6 +385,34 @@ rb_fiddle_ptr_free_get(VALUE self) return rb_fiddle_new_function(address, arg_types, ret_type); } +/* + * call-seq: free! => nil + * + * Call the free function for this pointer. Calling more than once will do + * nothing. + */ +static VALUE +rb_fiddle_ptr_free_bang(VALUE self) +{ + struct ptr_data *pdata; + TypedData_Get_Struct(self, struct ptr_data, &fiddle_ptr_data_type, pdata); + fiddle_ptr_free_ptr(pdata); + return Qnil; +} + +/* + * call-seq: freed? => bool + * + * Returns if this pointer has already been freed. + */ +static VALUE +rb_fiddle_ptr_freed_p(VALUE self) +{ + struct ptr_data *pdata; + TypedData_Get_Struct(self, struct ptr_data, &fiddle_ptr_data_type, pdata); + return pdata->freed ? Qtrue : Qfalse; +} + /* * call-seq: * @@ -711,6 +754,8 @@ Init_fiddle_pointer(void) rb_define_method(rb_cPointer, "initialize", rb_fiddle_ptr_initialize, -1); rb_define_method(rb_cPointer, "free=", rb_fiddle_ptr_free_set, 1); rb_define_method(rb_cPointer, "free", rb_fiddle_ptr_free_get, 0); + rb_define_method(rb_cPointer, "free!", rb_fiddle_ptr_free_bang, 0); + rb_define_method(rb_cPointer, "freed?", rb_fiddle_ptr_freed_p, 0); rb_define_method(rb_cPointer, "to_i", rb_fiddle_ptr_to_i, 0); rb_define_method(rb_cPointer, "to_int", rb_fiddle_ptr_to_i, 0); rb_define_method(rb_cPointer, "to_value", rb_fiddle_ptr_to_value, 0); diff --git a/test/fiddle/test_pointer.rb b/test/fiddle/test_pointer.rb index c69e4f71..0b3a3a56 100644 --- a/test/fiddle/test_pointer.rb +++ b/test/fiddle/test_pointer.rb @@ -20,16 +20,28 @@ def test_malloc_free_func_int assert_equal free.to_i, Fiddle::RUBY_FREE.to_i ptr = Pointer.malloc(10, free.to_i) - assert_equal 10, ptr.size - assert_equal free.to_i, ptr.free.to_i + refute ptr.freed? + begin + assert_equal 10, ptr.size + assert_equal free.to_i, ptr.free.to_i + ensure + ptr.free! + end + assert ptr.freed? end def test_malloc_free_func free = Fiddle::Function.new(Fiddle::RUBY_FREE, [TYPE_VOIDP], TYPE_VOID) ptr = Pointer.malloc(10, free) - assert_equal 10, ptr.size - assert_equal free.to_i, ptr.free.to_i + refute ptr.freed? + begin + assert_equal 10, ptr.size + assert_equal free.to_i, ptr.free.to_i + ensure + ptr.free! + end + assert ptr.freed? end def test_to_str @@ -85,16 +97,20 @@ def test_to_ptr_string def test_to_ptr_io buf = Pointer.malloc(10, Fiddle::RUBY_FREE) - File.open(__FILE__, 'r') do |f| - ptr = Pointer.to_ptr f - fread = Function.new(@libc['fread'], - [TYPE_VOIDP, TYPE_INT, TYPE_INT, TYPE_VOIDP], - TYPE_INT) - fread.call(buf.to_i, Fiddle::SIZEOF_CHAR, buf.size - 1, ptr.to_i) - end - - File.open(__FILE__, 'r') do |f| - assert_equal f.read(9), buf.to_s + begin + File.open(__FILE__, 'r') do |f| + ptr = Pointer.to_ptr f + fread = Function.new(@libc['fread'], + [TYPE_VOIDP, TYPE_INT, TYPE_INT, TYPE_VOIDP], + TYPE_INT) + fread.call(buf.to_i, Fiddle::SIZEOF_CHAR, buf.size - 1, ptr.to_i) + end + + File.open(__FILE__, 'r') do |f| + assert_equal f.read(9), buf.to_s + end + ensure + buf.free! end end @@ -170,6 +186,31 @@ def test_free= assert_equal free.ptr, ptr.free.ptr end + def test_free! + ptr = Pointer.malloc(4) + refute ptr.freed? + ptr.free! + assert ptr.freed? + ptr.free! # you can safely run it again + assert ptr.freed? + + ptr = Pointer.malloc(4, Fiddle::RUBY_FREE) + refute ptr.freed? + ptr.free! + assert ptr.freed? + ptr.free! # you can safely run it again + assert ptr.freed? + GC.start # you can safely run the GC routine + assert ptr.freed? + end + + def test_freed? + ptr = Pointer.malloc(4) + refute ptr.freed? + ptr.free! + assert ptr.freed? + end + def test_null? ptr = Pointer.new(0) assert ptr.null? From 270de771b3edcd205828081d440abcb1905fe7bd Mon Sep 17 00:00:00 2001 From: Chris Seaton Date: Tue, 2 Jun 2020 14:25:18 +0100 Subject: [PATCH 02/10] Use stdbool --- ext/fiddle/pointer.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index b9fb0fab..61bfa67d 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -2,6 +2,7 @@ * $Id$ */ +#include #include #include #include @@ -24,7 +25,7 @@ struct ptr_data { void *ptr; long size; freefunc_t free; - int freed; + bool freed; VALUE wrap[2]; }; @@ -64,7 +65,7 @@ fiddle_ptr_free_ptr(void *ptr) if (data->ptr && data->free && !data->freed) { (*(data->free))(data->ptr); } - data->freed = 1; + data->freed = true; } static void @@ -95,7 +96,7 @@ rb_fiddle_ptr_new2(VALUE klass, void *ptr, long size, freefunc_t func) val = TypedData_Make_Struct(klass, struct ptr_data, &fiddle_ptr_data_type, data); data->ptr = ptr; data->free = func; - data->freed = 0; + data->freed = false; data->size = size; return val; @@ -147,7 +148,7 @@ rb_fiddle_ptr_s_allocate(VALUE klass) data->ptr = 0; data->size = 0; data->free = 0; - data->freed = 0; + data->freed = false; return obj; } From cb4599735ca6c67dc9584d06da8e272244639051 Mon Sep 17 00:00:00 2001 From: Chris Seaton Date: Tue, 2 Jun 2020 14:43:54 +0100 Subject: [PATCH 03/10] Fiddle::Pointer#free! does nothing if no free function installed --- ext/fiddle/pointer.c | 2 +- test/fiddle/test_pointer.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index 61bfa67d..aca19122 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -63,9 +63,9 @@ fiddle_ptr_free_ptr(void *ptr) { struct ptr_data *data = ptr; if (data->ptr && data->free && !data->freed) { + data->freed = true; (*(data->free))(data->ptr); } - data->freed = true; } static void diff --git a/test/fiddle/test_pointer.rb b/test/fiddle/test_pointer.rb index 0b3a3a56..6ad23e5b 100644 --- a/test/fiddle/test_pointer.rb +++ b/test/fiddle/test_pointer.rb @@ -190,9 +190,9 @@ def test_free! ptr = Pointer.malloc(4) refute ptr.freed? ptr.free! - assert ptr.freed? + refute ptr.freed? ptr.free! # you can safely run it again - assert ptr.freed? + refute ptr.freed? ptr = Pointer.malloc(4, Fiddle::RUBY_FREE) refute ptr.freed? @@ -205,7 +205,7 @@ def test_free! end def test_freed? - ptr = Pointer.malloc(4) + ptr = Pointer.malloc(4, Fiddle::RUBY_FREE) refute ptr.freed? ptr.free! assert ptr.freed? From d9de7bcdc39e8eaacea52147d0c38f1544c392ad Mon Sep 17 00:00:00 2001 From: Chris Seaton Date: Tue, 2 Jun 2020 14:45:58 +0100 Subject: [PATCH 04/10] Tidy up Fiddle::Pointer specs --- test/fiddle/test_pointer.rb | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/test/fiddle/test_pointer.rb b/test/fiddle/test_pointer.rb index 6ad23e5b..d4620327 100644 --- a/test/fiddle/test_pointer.rb +++ b/test/fiddle/test_pointer.rb @@ -20,7 +20,6 @@ def test_malloc_free_func_int assert_equal free.to_i, Fiddle::RUBY_FREE.to_i ptr = Pointer.malloc(10, free.to_i) - refute ptr.freed? begin assert_equal 10, ptr.size assert_equal free.to_i, ptr.free.to_i @@ -186,14 +185,7 @@ def test_free= assert_equal free.ptr, ptr.free.ptr end - def test_free! - ptr = Pointer.malloc(4) - refute ptr.freed? - ptr.free! - refute ptr.freed? - ptr.free! # you can safely run it again - refute ptr.freed? - + def test_free_with_func ptr = Pointer.malloc(4, Fiddle::RUBY_FREE) refute ptr.freed? ptr.free! @@ -204,6 +196,15 @@ def test_free! assert ptr.freed? end + def test_free_with_no_func + ptr = Pointer.malloc(4) + refute ptr.freed? + ptr.free! + refute ptr.freed? + ptr.free! # you can safely run it again + refute ptr.freed? + end + def test_freed? ptr = Pointer.malloc(4, Fiddle::RUBY_FREE) refute ptr.freed? From 03f6f809102c344d5bdf060e36bc2e77a69e56d6 Mon Sep 17 00:00:00 2001 From: Chris Seaton Date: Tue, 2 Jun 2020 14:48:38 +0100 Subject: [PATCH 05/10] Improve Fiddle::Pointer docs --- ext/fiddle/pointer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index aca19122..54416d4c 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -390,7 +390,7 @@ rb_fiddle_ptr_free_get(VALUE self) * call-seq: free! => nil * * Call the free function for this pointer. Calling more than once will do - * nothing. + * nothing. Does nothing if there is no free function attached. */ static VALUE rb_fiddle_ptr_free_bang(VALUE self) @@ -404,7 +404,7 @@ rb_fiddle_ptr_free_bang(VALUE self) /* * call-seq: freed? => bool * - * Returns if this pointer has already been freed. + * Returns if the free function for this pointer has been called. */ static VALUE rb_fiddle_ptr_freed_p(VALUE self) From a5c64972796cbc8bab9013884c6bdd66b378fce4 Mon Sep 17 00:00:00 2001 From: Chris Seaton Date: Tue, 2 Jun 2020 23:42:34 +0100 Subject: [PATCH 06/10] Rename Fiddle::Pointer#free! to #call_free --- ext/fiddle/pointer.c | 6 +++--- test/fiddle/test_pointer.rb | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index 54416d4c..574b0fed 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -387,13 +387,13 @@ rb_fiddle_ptr_free_get(VALUE self) } /* - * call-seq: free! => nil + * call-seq: call_free => nil * * Call the free function for this pointer. Calling more than once will do * nothing. Does nothing if there is no free function attached. */ static VALUE -rb_fiddle_ptr_free_bang(VALUE self) +rb_fiddle_ptr_call_free(VALUE self) { struct ptr_data *pdata; TypedData_Get_Struct(self, struct ptr_data, &fiddle_ptr_data_type, pdata); @@ -755,7 +755,7 @@ Init_fiddle_pointer(void) rb_define_method(rb_cPointer, "initialize", rb_fiddle_ptr_initialize, -1); rb_define_method(rb_cPointer, "free=", rb_fiddle_ptr_free_set, 1); rb_define_method(rb_cPointer, "free", rb_fiddle_ptr_free_get, 0); - rb_define_method(rb_cPointer, "free!", rb_fiddle_ptr_free_bang, 0); + rb_define_method(rb_cPointer, "call_free", rb_fiddle_ptr_call_free, 0); rb_define_method(rb_cPointer, "freed?", rb_fiddle_ptr_freed_p, 0); rb_define_method(rb_cPointer, "to_i", rb_fiddle_ptr_to_i, 0); rb_define_method(rb_cPointer, "to_int", rb_fiddle_ptr_to_i, 0); diff --git a/test/fiddle/test_pointer.rb b/test/fiddle/test_pointer.rb index d4620327..c16fcd10 100644 --- a/test/fiddle/test_pointer.rb +++ b/test/fiddle/test_pointer.rb @@ -24,7 +24,7 @@ def test_malloc_free_func_int assert_equal 10, ptr.size assert_equal free.to_i, ptr.free.to_i ensure - ptr.free! + ptr.call_free end assert ptr.freed? end @@ -38,7 +38,7 @@ def test_malloc_free_func assert_equal 10, ptr.size assert_equal free.to_i, ptr.free.to_i ensure - ptr.free! + ptr.call_free end assert ptr.freed? end @@ -109,7 +109,7 @@ def test_to_ptr_io assert_equal f.read(9), buf.to_s end ensure - buf.free! + buf.call_free end end @@ -188,9 +188,9 @@ def test_free= def test_free_with_func ptr = Pointer.malloc(4, Fiddle::RUBY_FREE) refute ptr.freed? - ptr.free! + ptr.call_free assert ptr.freed? - ptr.free! # you can safely run it again + ptr.call_free # you can safely run it again assert ptr.freed? GC.start # you can safely run the GC routine assert ptr.freed? @@ -199,16 +199,16 @@ def test_free_with_func def test_free_with_no_func ptr = Pointer.malloc(4) refute ptr.freed? - ptr.free! + ptr.call_free refute ptr.freed? - ptr.free! # you can safely run it again + ptr.call_free # you can safely run it again refute ptr.freed? end def test_freed? ptr = Pointer.malloc(4, Fiddle::RUBY_FREE) refute ptr.freed? - ptr.free! + ptr.call_free assert ptr.freed? end From 25307ecb961129a46c09569e55f684125bbb46f8 Mon Sep 17 00:00:00 2001 From: Chris Seaton Date: Tue, 2 Jun 2020 23:44:02 +0100 Subject: [PATCH 07/10] Don't manually free pointers in tests for now --- test/fiddle/test_pointer.rb | 43 ++++++++++++------------------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/test/fiddle/test_pointer.rb b/test/fiddle/test_pointer.rb index c16fcd10..ee111778 100644 --- a/test/fiddle/test_pointer.rb +++ b/test/fiddle/test_pointer.rb @@ -20,27 +20,16 @@ def test_malloc_free_func_int assert_equal free.to_i, Fiddle::RUBY_FREE.to_i ptr = Pointer.malloc(10, free.to_i) - begin - assert_equal 10, ptr.size - assert_equal free.to_i, ptr.free.to_i - ensure - ptr.call_free - end - assert ptr.freed? + assert_equal 10, ptr.size + assert_equal free.to_i, ptr.free.to_i end def test_malloc_free_func free = Fiddle::Function.new(Fiddle::RUBY_FREE, [TYPE_VOIDP], TYPE_VOID) ptr = Pointer.malloc(10, free) - refute ptr.freed? - begin - assert_equal 10, ptr.size - assert_equal free.to_i, ptr.free.to_i - ensure - ptr.call_free - end - assert ptr.freed? + assert_equal 10, ptr.size + assert_equal free.to_i, ptr.free.to_i end def test_to_str @@ -96,20 +85,16 @@ def test_to_ptr_string def test_to_ptr_io buf = Pointer.malloc(10, Fiddle::RUBY_FREE) - begin - File.open(__FILE__, 'r') do |f| - ptr = Pointer.to_ptr f - fread = Function.new(@libc['fread'], - [TYPE_VOIDP, TYPE_INT, TYPE_INT, TYPE_VOIDP], - TYPE_INT) - fread.call(buf.to_i, Fiddle::SIZEOF_CHAR, buf.size - 1, ptr.to_i) - end - - File.open(__FILE__, 'r') do |f| - assert_equal f.read(9), buf.to_s - end - ensure - buf.call_free + File.open(__FILE__, 'r') do |f| + ptr = Pointer.to_ptr f + fread = Function.new(@libc['fread'], + [TYPE_VOIDP, TYPE_INT, TYPE_INT, TYPE_VOIDP], + TYPE_INT) + fread.call(buf.to_i, Fiddle::SIZEOF_CHAR, buf.size - 1, ptr.to_i) + end + + File.open(__FILE__, 'r') do |f| + assert_equal f.read(9), buf.to_s end end From 7c852e8b2a67ff1501cc17c760abb58472b0caf9 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Wed, 3 Jun 2020 09:18:52 +0900 Subject: [PATCH 08/10] free! -> call_free --- ext/fiddle/pointer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index 574b0fed..3e188f1d 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -209,7 +209,7 @@ rb_fiddle_ptr_initialize(int argc, VALUE argv[], VALUE self) * # Manually freeing but relying on the garbage collector otherwise * pointer = Fiddle::Pointer.malloc(size, Fiddle::RUBY_FREE) * ... - * pointer.free! + * pointer.call_free * * # Relying on the garbage collector - may lead to unlimited memory allocated before freeing any, but safe * pointer = Fiddle::Pointer.malloc(size, Fiddle::RUBY_FREE) From fd13082a3d1bc714a9ef9a3791bd88cdd9168126 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Wed, 3 Jun 2020 09:19:13 +0900 Subject: [PATCH 09/10] free! -> call_free --- ext/fiddle/pointer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index 3e188f1d..45a15d00 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -229,7 +229,7 @@ rb_fiddle_ptr_initialize(int argc, VALUE argv[], VALUE self) * * Allocate +size+ bytes of memory and associate it with an optional * +freefunc+ that will be called when the pointer is garbage collected, - * if not already manually called via *free!*. + * if not already manually called via *call_free*. * * +freefunc+ must be an address pointing to a function or an instance of * +Fiddle::Function+. Using +freefunc+ without using +free!+ may lead to From 6d6b1408ad93afb57cbe76d61b67091396ac5ff6 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Wed, 3 Jun 2020 09:19:38 +0900 Subject: [PATCH 10/10] free! -> call_free --- ext/fiddle/pointer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/fiddle/pointer.c b/ext/fiddle/pointer.c index 45a15d00..99d7f607 100644 --- a/ext/fiddle/pointer.c +++ b/ext/fiddle/pointer.c @@ -232,7 +232,7 @@ rb_fiddle_ptr_initialize(int argc, VALUE argv[], VALUE self) * if not already manually called via *call_free*. * * +freefunc+ must be an address pointing to a function or an instance of - * +Fiddle::Function+. Using +freefunc+ without using +free!+ may lead to + * +Fiddle::Function+. Using +freefunc+ without using +call_free+ 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