Merge pull request #1010 from casperisfine/pointer-write-barrier

Implement Write Barrier and dsize for FFI::Pointer

7 files changed, 104 insertions, 17 deletions

diff --git a/ext/ffi_c/AbstractMemory.c b/ext/ffi_c/AbstractMemory.c
index 8a4291a..f7fb295 100644
--- a/ext/ffi_c/AbstractMemory.c
+++ b/ext/ffi_c/AbstractMemory.c
@@ -55,6 +55,7 @@
 # define RB_OBJ_STRING(obj) StringValueCStr(obj)
 #endif
 
+static size_t memsize(const void *data);
 static inline char* memory_address(VALUE self);
 VALUE rbffi_AbstractMemoryClass = Qnil;
 static VALUE NullPointerErrorClass = Qnil;
@@ -65,9 +66,11 @@ const rb_data_type_t rbffi_abstract_memory_data_type = { /* extern */
     .function = {
         .dmark = NULL,
         .dfree = RUBY_TYPED_DEFAULT_FREE,
-        .dsize = NULL,
+        .dsize = 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 VALUE
@@ -80,6 +83,13 @@ memory_allocate(VALUE klass)
 
     return obj;
 }
+
+static size_t
+memsize(const void *data)
+{
+    return sizeof(AbstractMemory);
+}
+
 #define VAL(x, swap) (unlikely(((memory->flags & MEM_SWAP) != 0)) ? swap((x)) : (x))
 
 #define NUM_OP(name, type, toNative, fromNative, swap) \
diff --git a/ext/ffi_c/Buffer.c b/ext/ffi_c/Buffer.c
index e3ed90b..75f4873 100644
--- a/ext/ffi_c/Buffer.c
+++ b/ext/ffi_c/Buffer.c
@@ -52,16 +52,20 @@ static VALUE buffer_initialize(int argc, VALUE* argv, VALUE self);
 static void buffer_release(void *data);
 static void buffer_mark(void *data);
 static VALUE buffer_free(VALUE self);
+static size_t allocated_buffer_memsize(const void *data);
+static size_t buffer_memsize(const void *data);
 
 static const rb_data_type_t buffer_data_type = {
     .wrap_struct_name = "FFI::Buffer",
     .function = {
         .dmark = buffer_mark,
         .dfree = RUBY_TYPED_DEFAULT_FREE,
-        .dsize = NULL,
+        .dsize = buffer_memsize,
     },
     .parent = &rbffi_abstract_memory_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 const rb_data_type_t allocated_buffer_data_type = {
@@ -69,10 +73,12 @@ static const rb_data_type_t allocated_buffer_data_type = {
     .function = {
         .dmark = NULL,
         .dfree = buffer_release,
-        .dsize = NULL,
+        .dsize = allocated_buffer_memsize,
     },
     .parent = &buffer_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
 };
 
 
@@ -85,7 +91,7 @@ buffer_allocate(VALUE klass)
     VALUE obj;
 
     obj = TypedData_Make_Struct(klass, Buffer, &allocated_buffer_data_type, buffer);
-    buffer->data.rbParent = Qnil;
+    RB_OBJ_WRITE(obj, &buffer->data.rbParent, Qnil);
     buffer->memory.flags = MEM_RD | MEM_WR;
 
     return obj;
@@ -204,7 +210,7 @@ slice(VALUE self, long offset, long len)
     result->memory.size = len;
     result->memory.flags = ptr->memory.flags;
     result->memory.typeSize = ptr->memory.typeSize;
-    result->data.rbParent = self;
+    RB_OBJ_WRITE(obj, &result->data.rbParent, self);
 
     return obj;
 }
@@ -334,6 +340,23 @@ buffer_mark(void *data)
     rb_gc_mark(ptr->data.rbParent);
 }
 
+static size_t
+buffer_memsize(const void *data)
+{
+    return sizeof(Buffer);
+}
+
+static size_t
+allocated_buffer_memsize(const void *data)
+{
+    const Buffer *ptr = (const Buffer *)data;
+    size_t memsize = sizeof(Buffer);
+    if ((ptr->memory.flags & MEM_EMBED) == 0 && ptr->data.storage != NULL) {
+        memsize += ptr->memory.size;
+    }
+    return memsize;
+}
+
 void
 rbffi_Buffer_Init(VALUE moduleFFI)
 {
diff --git a/ext/ffi_c/MemoryPointer.c b/ext/ffi_c/MemoryPointer.c
index cfdfa60..0a747fb 100644
--- a/ext/ffi_c/MemoryPointer.c
+++ b/ext/ffi_c/MemoryPointer.c
@@ -40,6 +40,7 @@
 
 static VALUE memptr_allocate(VALUE klass);
 static void memptr_release(void *data);
+static size_t memptr_memsize(const void *data);
 static VALUE memptr_malloc(VALUE self, long size, long count, bool clear);
 static VALUE memptr_free(VALUE self);
 
@@ -58,10 +59,12 @@ static const rb_data_type_t memory_pointer_data_type = {
     .function = {
         .dmark = NULL,
         .dfree = memptr_release,
-        .dsize = NULL,
+        .dsize = memptr_memsize,
     },
     .parent = &rbffi_pointer_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 VALUE
@@ -69,7 +72,7 @@ memptr_allocate(VALUE klass)
 {
     Pointer* p;
     VALUE obj = TypedData_Make_Struct(klass, Pointer, &memory_pointer_data_type, p);
-    p->rbParent = Qnil;
+    RB_OBJ_WRITE(obj, &p->rbParent, Qnil);
     p->memory.flags = MEM_RD | MEM_WR;
 
     return obj;
@@ -157,6 +160,17 @@ memptr_release(void *data)
     xfree(ptr);
 }
 
+static size_t
+memptr_memsize(const void *data)
+{
+    const Pointer *ptr = (const Pointer *)data;
+    size_t memsize = sizeof(Pointer);
+    if (ptr->allocated) {
+        memsize += ptr->memory.size;
+    }
+    return memsize;
+}
+
 /*
  * call-seq: from_string(s)
  * @param [String] s string
diff --git a/ext/ffi_c/Pointer.c b/ext/ffi_c/Pointer.c
index 3d26555..cfa841e 100644
--- a/ext/ffi_c/Pointer.c
+++ b/ext/ffi_c/Pointer.c
@@ -43,16 +43,19 @@ VALUE rbffi_NullPointerSingleton = Qnil;
 
 static void ptr_release(void *data);
 static void ptr_mark(void *data);
+static size_t ptr_memsize(const void *data);
 
 const rb_data_type_t rbffi_pointer_data_type = { /* extern */
     .wrap_struct_name = "FFI::Pointer",
     .function = {
         .dmark = ptr_mark,
         .dfree = ptr_release,
-        .dsize = NULL,
+        .dsize = ptr_memsize,
     },
     .parent = &rbffi_abstract_memory_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
 };
 
 VALUE
@@ -70,7 +73,7 @@ rbffi_Pointer_NewInstance(void* addr)
     p->memory.size = LONG_MAX;
     p->memory.flags = (addr == NULL) ? 0 : (MEM_RD | MEM_WR);
     p->memory.typeSize = 1;
-    p->rbParent = Qnil;
+    RB_OBJ_WRITE(obj, &p->rbParent, Qnil);
 
     return obj;
 }
@@ -82,7 +85,7 @@ ptr_allocate(VALUE klass)
     VALUE obj;
 
     obj = TypedData_Make_Struct(klass, Pointer, &rbffi_pointer_data_type, p);
-    p->rbParent = Qnil;
+    RB_OBJ_WRITE(obj, &p->rbParent, Qnil);
     p->memory.flags = MEM_RD | MEM_WR;
 
     return obj;
@@ -134,7 +137,7 @@ ptr_initialize(int argc, VALUE* argv, VALUE self)
             if (rb_obj_is_kind_of(rbAddress, rbffi_PointerClass)) {
                 Pointer* orig;
 
-                p->rbParent = rbAddress;
+                RB_OBJ_WRITE(self, &p->rbParent, rbAddress);
                 TypedData_Get_Struct(rbAddress, Pointer, &rbffi_pointer_data_type, orig);
                 p->memory = orig->memory;
             } else {
@@ -215,7 +218,7 @@ slice(VALUE self, long offset, long size)
     p->memory.size = size;
     p->memory.flags = ptr->flags;
     p->memory.typeSize = ptr->typeSize;
-    p->rbParent = self;
+    RB_OBJ_WRITE(retval, &p->rbParent, self);
 
     return retval;
 }
@@ -471,6 +474,17 @@ ptr_mark(void *data)
     rb_gc_mark(ptr->rbParent);
 }
 
+static size_t
+ptr_memsize(const void *data)
+{
+    const Pointer *ptr = (const Pointer *)data;
+    size_t memsize = sizeof(Pointer);
+    if (ptr->allocated) {
+        memsize += ptr->memory.size;
+    }
+    return memsize;
+}
+
 void
 rbffi_Pointer_Init(VALUE moduleFFI)
 {
diff --git a/spec/ffi/buffer_spec.rb b/spec/ffi/buffer_spec.rb
index 619cb6b..e031f7c 100644
--- a/spec/ffi/buffer_spec.rb
+++ b/spec/ffi/buffer_spec.rb
@@ -285,3 +285,13 @@ describe "Buffer#initialize" do
     expect(block_executed).to be true
   end
 end
+
+describe "Buffer#memsize_of" do
+  it "has a memsize function", skip: RUBY_ENGINE != "ruby" do
+    base_size = ObjectSpace.memsize_of(Object.new)
+
+    buf = FFI::Buffer.new 14
+    size = ObjectSpace.memsize_of(buf)
+    expect(size).to be > base_size
+  end
+end
diff --git a/spec/ffi/pointer_spec.rb b/spec/ffi/pointer_spec.rb
index 9c85da7..c8242c7 100644
--- a/spec/ffi/pointer_spec.rb
+++ b/spec/ffi/pointer_spec.rb
@@ -381,5 +381,13 @@ describe "AutoPointer" do
       expect(mptr[1].read_uint).to eq(0xcafebabe)
     end
   end
+
+  it "has a memsize function", skip: RUBY_ENGINE != "ruby" do
+    base_size = ObjectSpace.memsize_of(Object.new)
+
+    pointer = FFI::Pointer.new(:int, 0xdeadbeef)
+    size = ObjectSpace.memsize_of(pointer)
+    expect(size).to be > base_size
+  end
 end
 
diff --git a/spec/ffi/rbx/memory_pointer_spec.rb b/spec/ffi/rbx/memory_pointer_spec.rb
index 3878973..2db89a9 100644
--- a/spec/ffi/rbx/memory_pointer_spec.rb
+++ b/spec/ffi/rbx/memory_pointer_spec.rb
@@ -189,4 +189,12 @@ describe "MemoryPointer" do
     end
     expect(block_executed).to be true
   end
+
+  it "has a memsize function", skip: RUBY_ENGINE != "ruby" do
+    base_size = ObjectSpace.memsize_of(Object.new)
+
+    pointer = FFI::MemoryPointer.from_string("FFI is Awesome")
+    size = ObjectSpace.memsize_of(pointer)
+    expect(size).to be > base_size
+  end
 end