Allow private release method for ManagedStruct and AutoPointer

FFI users might not want to expose `release` as a public class method, but want to keep it private.

Also re-enable GC tests that were disabled as part of #427, but make them opt-in per env var FFI_TEST_GC.

Derivations of Releaser are no longer necessary, so that they are removed now.

6 files changed, 18 insertions, 27 deletions

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 58fdbd0..371d372 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -32,7 +32,7 @@ jobs:
       env:
         # work around misconfiguration of libffi on MacOS with homebrew
         PKG_CONFIG_PATH: ${{ env.PKG_CONFIG_PATH }}:/usr/local/opt/libffi/lib/pkgconfig
-    - run: bundle exec rake test
+    - run: bundle exec rake test FFI_TEST_GC=true
     - run: bundle exec rake types_conf && git --no-pager diff
 
   specs:
@@ -62,7 +62,7 @@ jobs:
     - run: bundle exec rake libffi
     - run: bundle exec rake compile
 
-    - run: bundle exec rake test
+    - run: bundle exec rake test FFI_TEST_GC=true
 
     - run: bundle exec rake bench:all
       if: ${{ matrix.ruby != 'truffleruby-head' && matrix.ruby != 'jruby-head' }}
diff --git a/lib/ffi/autopointer.rb b/lib/ffi/autopointer.rb
index 6c6333c..4f44171 100644
--- a/lib/ffi/autopointer.rb
+++ b/lib/ffi/autopointer.rb
@@ -84,13 +84,13 @@ module FFI
                     if not proc.respond_to?(:call)
                       raise RuntimeError.new("proc must be callable")
                     end
-                    CallableReleaser.new(ptr, proc)
+                    Releaser.new(ptr, proc)
 
                   else
-                    if not self.class.respond_to?(:release)
+                    if not self.class.respond_to?(:release, true)
                       raise RuntimeError.new("no release method defined")
                     end
-                    DefaultReleaser.new(ptr, self.class)
+                    Releaser.new(ptr, self.class.method(:release))
                   end
 
       ObjectSpace.define_finalizer(self, @releaser)
@@ -149,23 +149,7 @@ module FFI
       def call(*args)
         release(@ptr) if @autorelease && @ptr
       end
-    end
-
-    # DefaultReleaser is a {Releaser} used when an {AutoPointer} is defined
-    # without Proc or Method. In this case, the pointer to release must be of
-    # a class derived from AutoPointer with a {release} class method.
-    class DefaultReleaser < Releaser
-      # @param [Pointer] ptr
-      # @return [nil]
-      # Release +ptr+ using the {release} class method of its class.
-      def release(ptr)
-        @proc.release(ptr)
-      end
-    end
 
-    # CallableReleaser is a {Releaser} used when an {AutoPointer} is defined with a
-    # Proc or a Method.
-    class CallableReleaser < Releaser
       # Release +ptr+ by using Proc or Method defined at +ptr+
       # {AutoPointer#initialize initialization}.
       #
@@ -182,7 +166,7 @@ module FFI
     # @return [Type::POINTER]
     # @raise {RuntimeError} if class does not implement a +#release+ method
     def self.native_type
-      if not self.respond_to?(:release)
+      if not self.respond_to?(:release, true)
         raise RuntimeError.new("no release method defined for #{self.inspect}")
       end
       Type::POINTER
diff --git a/lib/ffi/managedstruct.rb b/lib/ffi/managedstruct.rb
index b5ec8a3..5d243e5 100644
--- a/lib/ffi/managedstruct.rb
+++ b/lib/ffi/managedstruct.rb
@@ -75,7 +75,7 @@ module FFI
     # @overload initialize
     # A new instance of FFI::ManagedStruct.
     def initialize(pointer=nil)
-      raise NoMethodError, "release() not implemented for class #{self}" unless self.class.respond_to? :release
+      raise NoMethodError, "release() not implemented for class #{self}" unless self.class.respond_to?(:release, true)
       raise ArgumentError, "Must supply a pointer to memory for the Struct" unless pointer
       super AutoPointer.new(pointer, self.class.method(:release))
     end
diff --git a/spec/ffi/managed_struct_spec.rb b/spec/ffi/managed_struct_spec.rb
index b4e80bb..6299bdc 100644
--- a/spec/ffi/managed_struct_spec.rb
+++ b/spec/ffi/managed_struct_spec.rb
@@ -46,6 +46,8 @@ describe "Managed Struct" do
       def self.release(_ptr)
         @@count += 1
       end
+      private_class_method :release
+
       def self.wait_gc(count)
         loop = 5
         while loop > 0 && @@count < count
diff --git a/spec/ffi/pointer_spec.rb b/spec/ffi/pointer_spec.rb
index c8242c7..820e144 100644
--- a/spec/ffi/pointer_spec.rb
+++ b/spec/ffi/pointer_spec.rb
@@ -257,6 +257,7 @@ describe "AutoPointer" do
     def self.release
       @@count += 1 if @@count > 0
     end
+    private_class_method(:release)
     def self.reset
       @@count = 0
     end
@@ -275,10 +276,11 @@ describe "AutoPointer" do
   end
   class AutoPointerSubclass < FFI::AutoPointer
     def self.release(ptr); end
+    private_class_method(:release)
   end
 
   # see #427
-  it "cleanup via default release method", :broken => true do
+  it "cleanup via default release method", gc_dependent: true do
     expect(AutoPointerSubclass).to receive(:release).at_least(loop_count-wiggle_room).times
     AutoPointerTestHelper.reset
     loop_count.times do
@@ -291,7 +293,7 @@ describe "AutoPointer" do
   end
 
   # see #427
-  it "cleanup when passed a proc", :broken => true do
+  it "cleanup when passed a proc", gc_dependent: true do
     #  NOTE: passing a proc is touchy, because it's so easy to create a memory leak.
     #
     #  specifically, if we made an inline call to
@@ -310,7 +312,7 @@ describe "AutoPointer" do
   end
 
   # see #427
-  it "cleanup when passed a method", :broken => true do
+  it "cleanup when passed a method", gc_dependent: true do
     expect(AutoPointerTestHelper).to receive(:release).at_least(loop_count-wiggle_room).times
     AutoPointerTestHelper.reset
     loop_count.times do
@@ -327,6 +329,7 @@ describe "AutoPointer" do
         ffi_lib TestLibrary::PATH
         class CustomAutoPointer < FFI::AutoPointer
           def self.release(ptr); end
+          private_class_method(:release)
         end
         attach_function :ptr_from_address, [ FFI::Platform::ADDRESS_SIZE == 32 ? :uint : :ulong_long ], CustomAutoPointer
       end
@@ -349,6 +352,7 @@ describe "AutoPointer" do
   describe "#autorelease?" do
     ptr_class = Class.new(FFI::AutoPointer) do
       def self.release(ptr); end
+      private_class_method(:release)
     end
 
     it "should be true by default" do
@@ -365,6 +369,7 @@ describe "AutoPointer" do
   describe "#type_size" do
     ptr_class = Class.new(FFI::AutoPointer) do
       def self.release(ptr); end
+      private_class_method(:release)
     end
 
     it "type_size of AutoPointer should match wrapped Pointer" do
diff --git a/spec/ffi/spec_helper.rb b/spec/ffi/spec_helper.rb
index e9ced3d..a0adbed 100644
--- a/spec/ffi/spec_helper.rb
+++ b/spec/ffi/spec_helper.rb
@@ -8,7 +8,7 @@ require 'timeout'
 require 'objspace'
 
 RSpec.configure do |c|
-  c.filter_run_excluding :broken => true
+  c.filter_run_excluding gc_dependent: true unless ENV['FFI_TEST_GC'] == 'true'
 end
 
 module TestLibrary