Merge pull request #1005 from casperisfine/type-write-barrier

Implement Write Barrier and dsize for FFI::Type

5 files changed, 83 insertions, 23 deletions

diff --git a/ext/ffi_c/MappedType.c b/ext/ffi_c/MappedType.c
index 304de86..ed05c9d 100644
--- a/ext/ffi_c/MappedType.c
+++ b/ext/ffi_c/MappedType.c
@@ -39,6 +39,7 @@
 static VALUE mapped_allocate(VALUE);
 static VALUE mapped_initialize(VALUE, VALUE);
 static void mapped_mark(void *);
+static size_t mapped_memsize(const void *);
 static ID id_native_type, id_to_native, id_from_native;
 
 VALUE rbffi_MappedTypeClass = Qnil;
@@ -48,10 +49,12 @@ static const rb_data_type_t mapped_type_data_type = {
   .function = {
       .dmark = mapped_mark,
       .dfree = RUBY_TYPED_DEFAULT_FREE,
-      .dsize = NULL,
+      .dsize = mapped_memsize,
   },
   .parent = &rbffi_type_data_type,
-  .flags = RUBY_TYPED_FREE_IMMEDIATELY
+  // IMPORTANT: WB_PROTECTED objects must only use the RB_OBJ_WRITE()
+  // macro to update VALUE references, as to trigger write barriers.
+  .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED
 };
 
 
@@ -62,8 +65,8 @@ mapped_allocate(VALUE klass)
 
     VALUE obj = TypedData_Make_Struct(klass, MappedType, &mapped_type_data_type, m);
 
-    m->rbConverter = Qnil;
-    m->rbType = Qnil;
+    RB_OBJ_WRITE(obj, &m->rbConverter, Qnil);
+    RB_OBJ_WRITE(obj, &m->rbType, Qnil);
     m->type = NULL;
     m->base.nativeType = NATIVE_MAPPED;
     m->base.ffiType = &ffi_type_void;
@@ -95,12 +98,12 @@ mapped_initialize(VALUE self, VALUE rbConverter)
     }
 
     TypedData_Get_Struct(self, MappedType, &mapped_type_data_type, m);
-    m->rbType = rb_funcall2(rbConverter, id_native_type, 0, NULL);
+    RB_OBJ_WRITE(self, &m->rbType, rb_funcall2(rbConverter, id_native_type, 0, NULL));
     if (!(rb_obj_is_kind_of(m->rbType, rbffi_TypeClass))) {
         rb_raise(rb_eTypeError, "native_type did not return instance of FFI::Type");
     }
 
-    m->rbConverter = rbConverter;
+    RB_OBJ_WRITE(self, &m->rbConverter, rbConverter);
     TypedData_Get_Struct(m->rbType, Type, &rbffi_type_data_type, m->type);
     m->base.ffiType = m->type->ffiType;
 
@@ -115,6 +118,12 @@ mapped_mark(void* data)
     rb_gc_mark(m->rbConverter);
 }
 
+static size_t
+mapped_memsize(const void *data)
+{
+    return sizeof(MappedType);
+}
+
 /*
  * call-seq: mapped_type.native_type
  * @return [Type]
diff --git a/ext/ffi_c/MappedType.h b/ext/ffi_c/MappedType.h
index ac86a3c..9f6f9ee 100644
--- a/ext/ffi_c/MappedType.h
+++ b/ext/ffi_c/MappedType.h
@@ -43,7 +43,6 @@ typedef struct MappedType_ {
     Type* type;
     VALUE rbConverter;
     VALUE rbType;
-    
 } MappedType;
 
 void rbffi_MappedType_Init(VALUE moduleFFI);
diff --git a/ext/ffi_c/Type.c b/ext/ffi_c/Type.c
index d940e94..6089a14 100644
--- a/ext/ffi_c/Type.c
+++ b/ext/ffi_c/Type.c
@@ -39,13 +39,14 @@
 #include "Types.h"
 #include "Type.h"
 
+static size_t type_memsize(const void *);
 
 typedef struct BuiltinType_ {
     Type type;
-    char* name;
+    const char* name;
 } BuiltinType;
 
-static void builtin_type_free(void *);
+static size_t builtin_type_memsize(const void *);
 
 VALUE rbffi_TypeClass = Qnil;
 
@@ -59,22 +60,32 @@ const rb_data_type_t rbffi_type_data_type = { /* extern */
   .function = {
       .dmark = NULL,
       .dfree = RUBY_TYPED_DEFAULT_FREE,
-      .dsize = NULL,
+      .dsize = type_memsize,
   },
-  .flags = RUBY_TYPED_FREE_IMMEDIATELY
+  // IMPORTANT: WB_PROTECTED objects must only use the RB_OBJ_WRITE()
+  // macro to update VALUE references, as to trigger write barriers.
+  .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED
 };
 
 static const rb_data_type_t builtin_type_data_type = {
   .wrap_struct_name = "FFI::Type::Builtin",
   .function = {
       .dmark = NULL,
-      .dfree = builtin_type_free,
-      .dsize = NULL,
+      .dfree = RUBY_TYPED_DEFAULT_FREE,
+      .dsize = builtin_type_memsize,
   },
   .parent = &rbffi_type_data_type,
-  .flags = RUBY_TYPED_FREE_IMMEDIATELY
+  // IMPORTANT: WB_PROTECTED objects must only use the RB_OBJ_WRITE()
+  // macro to update VALUE references, as to trigger write barriers.
+  .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED
 };
 
+static size_t
+type_memsize(const void *data)
+{
+    return sizeof(Type);
+}
+
 static VALUE
 type_allocate(VALUE klass)
 {
@@ -83,7 +94,7 @@ type_allocate(VALUE klass)
 
     type->nativeType = -1;
     type->ffiType = &ffi_type_void;
-    
+
     return obj;
 }
 
@@ -170,20 +181,18 @@ builtin_type_new(VALUE klass, int nativeType, ffi_type* ffiType, const char* nam
     VALUE obj = Qnil;
 
     obj = TypedData_Make_Struct(klass, BuiltinType, &builtin_type_data_type, type);
-    
-    type->name = strdup(name);
+
+    type->name = name;
     type->type.nativeType = nativeType;
     type->type.ffiType = ffiType;
 
     return obj;
 }
 
-static void
-builtin_type_free(void *data)
+static size_t
+builtin_type_memsize(const void *data)
 {
-    BuiltinType *type = (BuiltinType *)data;
-    free(type->name);
-    xfree(type);
+    return sizeof(BuiltinType) + sizeof(ffi_type);
 }
 
 /*
diff --git a/ext/ffi_c/Type.h b/ext/ffi_c/Type.h
index 74a931b..b6de634 100644
--- a/ext/ffi_c/Type.h
+++ b/ext/ffi_c/Type.h
@@ -44,7 +44,7 @@ extern "C" {
 typedef struct Type_ Type;
 
 #include "Types.h"
-    
+
 struct Type_ {
     NativeType nativeType;
     ffi_type* ffiType;
diff --git a/spec/ffi/type_spec.rb b/spec/ffi/type_spec.rb
new file mode 100644
index 0000000..eb48a43
--- /dev/null
+++ b/spec/ffi/type_spec.rb
@@ -0,0 +1,43 @@
+#
+# This file is part of ruby-ffi.
+# For licensing, see LICENSE.SPECS
+#
+
+require File.expand_path(File.join(File.dirname(__FILE__), "spec_helper"))
+
+describe "FFI::Type" do
+  it 'has a memsize function', skip: RUBY_ENGINE != "ruby" do
+    base_size = ObjectSpace.memsize_of(Object.new)
+
+    size = ObjectSpace.memsize_of(FFI::Type.new(42))
+    expect(size).to be > base_size
+    base_size = size
+
+    converter = Module.new do
+      extend FFI::DataConverter
+
+      def self.native_type
+        @native_type_called = true
+        FFI::Type::INT32
+      end
+
+      def self.to_native(val, ctx)
+        @to_native_called = true
+        ToNativeMap[val]
+      end
+
+      def self.from_native(val, ctx)
+        @from_native_called = true
+        FromNativeMap[val]
+      end
+    end
+
+    size = ObjectSpace.memsize_of(FFI::Type::Mapped.new(converter))
+    expect(size).to be > base_size
+    base_size = size
+
+    # Builtin types are larger as they also have a name and own ffi_type
+    size = ObjectSpace.memsize_of(FFI::Type::Builtin::CHAR)
+    expect(size).to be > base_size
+  end
+end