Fixed FFI::MemoryPointer#autorelease= and added #autorelease?

autorelease= false was previously a noop, leaving the finalizer in place. Fixes #3260
rubinius · Dec 30, 2014 · 010342a · 010342a
1 parent eec1ad3
commit 010342a
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 6 deletions.
diff --git a/kernel/platform/pointer.rb b/kernel/platform/pointer.rb
@@ -379,6 +379,13 @@ def autorelease=(val)
       raise PrimitiveFailure, "FFI::MemoryPointer#autorelease= primitive failed"
     end
 
+    ##
+    # Returns true if autorelease is enabled, otherwise false.
+    def autorelease?
+      Rubinius.primitive :pointer_autorelease_p
+      raise PrimitiveFailure, "FFI::MemoryPointer#pointer_autorelease_p primitive failed"
+    end
+
   end
 
   class DynamicLibrary::Symbol < Pointer

diff --git a/spec/core/ffi/memorypointer/autorelease_spec.rb b/spec/core/ffi/memorypointer/autorelease_spec.rb
@@ -1,6 +1,32 @@
 require File.expand_path('../../../../spec_helper', __FILE__)
 require 'rubinius/ffi'
 
-describe "FFI::MemoryPointer#autorelease=" do
-  it "needs to be reviewed for spec completeness"
+describe "FFI::MemoryPointer#autorelease" do
+  before :all do
+    @libc = Module.new
+    @libc.extend Rubinius::FFI::Library
+    @libc.ffi_lib Rubinius::FFI::Library::LIBC
+    @libc.attach_function :free, [:pointer], :void
+  end
+
+  it "starts as enabled" do
+    ptr = Rubinius::FFI::MemoryPointer.new :int
+    ptr.autorelease?.should == true
+  end
+
+  it "can be disabled (and the pointer freed explicitly)" do
+    ptr = Rubinius::FFI::MemoryPointer.new :int
+    ptr.autorelease = false
+    ptr.autorelease?.should == false
+    @libc.free ptr
+  end
+
+  it "can be disabled and reenabled" do
+    ptr = Rubinius::FFI::MemoryPointer.new :int
+    ptr.autorelease?.should == true
+    ptr.autorelease = false
+    ptr.autorelease?.should == false
+    ptr.autorelease = true
+    ptr.autorelease?.should == true
+  end
 end
diff --git a/vm/builtin/ffi_pointer.cpp b/vm/builtin/ffi_pointer.cpp
@@ -106,13 +106,24 @@ namespace rubinius {
     return Pointer::create(state, (char*)pointer + amount->to_native());
   }
 
+  Object* Pointer::autorelease_p(STATE) {
+    return set_finalizer ? cTrue : cFalse;
+  }
+
   Object* Pointer::set_autorelease(STATE, Object* val) {
     autorelease = val->true_p() ? true : false;
 
-    if(autorelease && !set_finalizer) {
-      state->memory()->needs_finalization(this,
-          (FinalizerFunction)&Pointer::finalize);
-      set_finalizer = true;
+    if(autorelease) {
+      if(!set_finalizer) {
+        state->memory()->needs_finalization(this,
+            (FinalizerFunction)&Pointer::finalize);
+        set_finalizer = true;
+      }
+    } else {
+      if(set_finalizer) {
+        state->memory()->set_ruby_finalizer(this, cNil);
+        set_finalizer = false;
+      }
     }
 
     return val;

diff --git a/vm/builtin/ffi_pointer.hpp b/vm/builtin/ffi_pointer.hpp
@@ -37,6 +37,9 @@ namespace rubinius {
     // Rubinius.primitive :pointer_free
     Object* free(STATE);
 
+    // Rubinius.primitive :pointer_autorelease_p
+    Object* autorelease_p(STATE);
+
     // Rubinius.primitive :pointer_set_autorelease
     Object* set_autorelease(STATE, Object* val);