Merge pull request #2 from stormbreakerbg/managed-external-objects

Managed external objects

ext/v8/external.cc

@@ -0,0 +1,43 @@
+#include "rr.h"
+#include "external.h"
+
+namespace rr {
+  void External::Init() {
+    ClassBuilder("External").
+      defineSingletonMethod("New", &New).
+
+      defineMethod("Value", &Value).
+
+      store(&Class);
+  }
+
+  VALUE External::New(VALUE self, VALUE r_isolate, VALUE object) {
+    Isolate isolate(r_isolate);
+
+    // as long as this external is alive within JavaScript, it should not be
+    // garbage collected by Ruby.
+    isolate.retainObject(object);
+
+    Locker lock(isolate);
+
+    // create the external.
+    Container* container = new Container(object);
+    v8::Local<v8::External> external(v8::External::New(isolate, (void*)container));
+
+    // next, we create a weak reference to this external so that we can be
+    // notified when V8 is done with it. At that point, we can let Ruby know
+    // that this external is done with it.
+    v8::Global<v8::External>* global(new v8::Global<v8::External>(isolate, external));
+    global->SetWeak<Container>(container, &release, v8::WeakCallbackType::kParameter);
+    container->global = global;
+
+    return External(isolate, external);
+  }
+
+  VALUE External::Value(VALUE self) {
+    External external(self);
+    Locker lock(external);
+    Container* container((Container*)external->Value());
+    return container->object;
+  }
+}

ext/v8/external.h

@@ -0,0 +1,46 @@
+// -*- mode: c++ -*-
+#ifndef EXTERNAL_H
+#define EXTERNAL_H
+
+namespace rr {
+  class External : Ref<v8::External> {
+  public:
+
+    static void Init();
+    static VALUE New(VALUE self, VALUE isolate, VALUE object);
+    static VALUE Value(VALUE self);
+
+    inline External(VALUE value) : Ref<v8::External>(value) {}
+    inline External(v8::Isolate* isolate, v8::Handle<v8::External> handle) :
+      Ref<v8::External>(isolate, handle) {}
+
+    struct Container {
+      Container(VALUE v) : object(v) {}
+
+      v8::Global<v8::External>* global;
+      VALUE object;
+    };
+
+    /**
+     * Implements a v8::WeakCallbackInfo<Container>::Callback with all
+     * of its idiosyncracies. It happens in two passes. In the first
+     * pass, you are required to only reset the weak reference. In the
+     * second pass, you can actually do your cleanup. In this case, we
+     * schedule the referenced Ruby object to be released in the next
+     * Ruby gc pass.
+     */
+    static void release(const v8::WeakCallbackInfo<Container>& info) {
+      Container* container(info.GetParameter());
+      if (info.IsFirstPass()) {
+        container->global->Reset();
+        info.SetSecondPassCallback(&release);
+      } else {
+        Isolate isolate(info.GetIsolate());
+        isolate.scheduleReleaseObject(container->object);
+        delete container;
+      }
+    }
+  };
+}
+
+#endif /* EXTERNAL_H */

ext/v8/init.cc

@@ -20,6 +20,7 @@ extern "C" {
     Function::Init();
     Script::Init();
     Array::Init();
+    External::Init();
 
     // Accessor::Init();
     // Invocation::Init();

ext/v8/isolate.cc

@@ -5,6 +5,7 @@
 namespace rr {
 
   void Isolate::Init() {
+    rb_eval_string("require 'v8/retained_objects'");
     ClassBuilder("Isolate").
       defineSingletonMethod("New", &New).
 
@@ -17,11 +18,16 @@ namespace rr {
 
   VALUE Isolate::New(VALUE self) {
     Isolate::IsolateData* data = new IsolateData();
+    VALUE rb_cRetainedObjects = rb_eval_string("V8::RetainedObjects");
+    data->retained_objects = rb_funcall(rb_cRetainedObjects, rb_intern("new"), 0);
+
     v8::Isolate::CreateParams create_params;
     create_params.array_buffer_allocator = &data->array_buffer_allocator;
+
     Isolate isolate(v8::Isolate::New(create_params));
-    isolate->SetData(0, new IsolateData());
+    isolate->SetData(0, data);
     isolate->AddGCPrologueCallback(&clearReferences);
+
     return isolate;
   }
 

ext/v8/isolate.h

@@ -38,8 +38,8 @@ namespace rr {
      * its book keeping data. E.g.
      * VALUE rubyObject = Isolate(v8::Isolate::New());
      */
-    inline operator VALUE() {
+    virtual operator VALUE() {
-      return Data_Wrap_Struct(Class, 0, 0, pointer);
+      return Data_Wrap_Struct(Class, &releaseAndMarkRetainedObjects, 0, pointer);
     }
 
     /**
@@ -53,11 +53,61 @@ namespace rr {
 
     /**
      * Schedule a v8::Persistent reference to be be deleted with the next
-     * invocation of the V8 Garbarge Collector
+     * invocation of the V8 Garbarge Collector. It is safe to call
+     * this method from within the Ruby garbage collection thread or a place
+     * where you do not want to acquire any V8 locks.
      */
     template <class T>
-    inline void scheduleDelete(v8::Persistent<T>* cell) {
+    inline void scheduleReleaseObject(v8::Persistent<T>* cell) {
-      data()->queue.enqueue((v8::Persistent<void>*)cell);
+      data()->v8_release_queue.enqueue((v8::Persistent<void>*)cell);
+    }
+
+    /**
+     * Schedule a Ruby object to be released with the next invocation
+     * of the Ruby garbage collector. This method is safe to call from places
+     * where you do not hold any Ruby locks (such as the V8 GC thread)
+     */
+    inline void scheduleReleaseObject(VALUE object) {
+      data()->rb_release_queue.enqueue(object);
+    }
+
+    /**
+     * Increase the reference count to this Ruby object by one. As
+     * long as there is more than 1 reference to this object, it will
+     * not be garbage collected, even if there are no references to
+     * from within Ruby code.
+     *
+     * Note: should be called from a place where Ruby locks are held.
+     */
+    inline void retainObject(VALUE object) {
+      rb_funcall(data()->retained_objects, rb_intern("add"), 1, object);
+    }
+
+    /**
+     * Decrease the reference count to this Ruby object by one. If the
+     * count falls below zero, this object will no longer be marked my
+     * this Isolate and will be eligible for garbage collection.
+     *
+     * Note: should be called from a place where Ruby locks are held.
+     */
+    inline void releaseObject(VALUE object) {
+      rb_funcall(data()->retained_objects, rb_intern("remove"), 1, object);
+    }
+
+
+    /**
+     * The `gc_mark()` callback for this Isolate's
+     * Data_Wrap_Struct. It releases all pending Ruby objects.
+     */
+
+    static void releaseAndMarkRetainedObjects(v8::Isolate* isolate_) {
+      Isolate isolate(isolate_);
+      IsolateData* data = isolate.data();
+      VALUE object;
+      while (data->rb_release_queue.try_dequeue(object)) {
+        isolate.releaseObject(object);
+      }
+      rb_gc_mark(data->retained_objects);
     }
 
     /**
@@ -68,7 +118,7 @@ namespace rr {
     static void clearReferences(v8::Isolate* i, v8::GCType type, v8::GCCallbackFlags flags) {
       Isolate isolate(i);
       v8::Persistent<void>* cell;
-      while (isolate.data()->queue.try_dequeue(cell)) {
+      while (isolate.data()->v8_release_queue.try_dequeue(cell)) {
         cell->Reset();
         delete cell;
       }
@@ -97,6 +147,14 @@ namespace rr {
      * as the isolate.
      */
     struct IsolateData {
+
+      /**
+       * An instance of `V8::RetainedObjects` that contains all
+       * references held from from V8. As long as they are in this
+       * list, they won't be gc'd by Ruby.
+       */
+      VALUE retained_objects;
+
       /**
        * A custom ArrayBufferAllocator for this isolate. Why? because
        * if you don't, it will segfault when you try and create an
@@ -109,7 +167,15 @@ namespace rr {
        * finished with an object it will be enqueued here so that it
        * can be released by the v8 garbarge collector later.
        */
-      ConcurrentQueue<v8::Persistent<void>*> queue;
+      ConcurrentQueue<v8::Persistent<void>*> v8_release_queue;
+
+      /**
+       * Ruby objects that had been retained by this isolate, but that
+       * are eligible for release. Generally, an object ends up in a
+       * queue when the v8 object that had referenced it no longer
+       * needs it.
+       */
+      ConcurrentQueue<VALUE> rb_release_queue;
     };
   };
 }

ext/v8/ref.h

@@ -75,6 +75,10 @@ namespace rr {
       return isolate;
     }
 
+    inline operator v8::Isolate*() const {
+      return isolate;
+    }
+
     static void destroy(Holder* holder) {
       delete holder;
     }

ext/v8/rr.h

@@ -37,6 +37,7 @@ inline VALUE not_implemented(const char* message) {
 #include "object.h"
 #include "array.h"
 #include "primitive.h"
+#include "external.h"
 // This one is named v8_string to avoid name collisions with C's string.h
 #include "rr_string.h"
 

lib/v8/retained_objects.rb

@@ -0,0 +1,29 @@
+module V8
+  class RetainedObjects
+    def initialize
+      @counts = {}
+    end
+
+    def add(object)
+      if @counts[object]
+        @counts[object] += 1
+      else
+        @counts[object] = 1
+      end
+    end
+
+    def remove(object)
+      if count = @counts[object]
+        if count <= 1
+          @counts.delete object
+        else
+          @counts[object] -= 1
+        end
+      end
+    end
+
+    def retaining?(object)
+      !!@counts[object]
+    end
+  end
+end

spec/c/external_spec.rb

@@ -0,0 +1,23 @@
+require 'c_spec_helper'
+
+describe V8::C::External do
+  let(:isolate) { V8::C::Isolate::New() }
+  let(:value) { @external::Value() }
+  around { |example| V8::C::HandleScope(isolate) { example.run } }
+
+  before do
+    Object.new.tap do |object|
+      @object_id = object.object_id
+      @external = V8::C::External::New(isolate, object)
+    end
+  end
+
+  it "exists" do
+    expect(@external).to be
+  end
+
+  it "can retrieve the ruby object out from V8 land" do
+    expect(value).to be
+    expect(value.object_id).to eql @object_id
+  end
+end

spec/v8/retained_objects_spec.rb

@@ -0,0 +1,47 @@
+require 'v8/retained_objects'
+
+describe V8::RetainedObjects do
+  let(:object) { Object.new }
+  let(:objects) { V8::RetainedObjects.new }
+
+  it "knows that something isn't retained" do
+    expect(objects).not_to be_retaining object
+  end
+
+  describe "adding a reference to an object" do
+    before do
+      objects.add(object)
+    end
+
+    it "is now retained" do
+      expect(objects).to be_retaining object
+    end
+
+    describe "removing the reference" do
+      before do
+        objects.remove(object)
+      end
+      it "is no longer retained" do
+        expect(objects).to_not be_retaining object
+      end
+    end
+    describe "adding another reference and then removing" do
+      before do
+        objects.add(object)
+        objects.remove(object)
+      end
+      it "is still retained" do
+        expect(objects).to be_retaining object
+      end
+
+      describe "removing one more time" do
+        before do
+          objects.remove(object)
+        end
+        it "is no longer retained" do
+          expect(objects).to_not be_retaining object
+        end
+      end
+    end
+  end
+end