diff --git a/ext/v8/null.h b/ext/v8/null.h index 8912e1e..a8acd62 100644 --- a/ext/v8/null.h +++ b/ext/v8/null.h @@ -5,12 +5,20 @@ namespace rr { class Null : public Ref { public: - Null(v8::Isolate* isolate, v8::Local undefined) : - Ref(isolate, undefined) {} - static inline void Init() { ClassBuilder("Null", Primitive::Class). store(&Class); + + //implement V8::C::Null(isolate) + rb_define_singleton_method(rb_eval_string("V8::C"), "Null", (VALUE (*)(...))&instance, 1); + } + Null(v8::Isolate* isolate, v8::Local undefined) : + Ref(isolate, undefined) {} + + static VALUE instance(VALUE self, VALUE rb_isolate) { + Isolate isolate(rb_isolate); + Locker lock(isolate); + return Null(isolate, v8::Null(isolate)); } }; } diff --git a/lib/v8/conversion.rb b/lib/v8/conversion.rb index 6fb3af9..bc6f4ed 100644 --- a/lib/v8/conversion.rb +++ b/lib/v8/conversion.rb @@ -119,6 +119,12 @@ module V8::C end end +class NilClass + def to_v8(context) + V8::C::Null(context.isolate.native) + end +end + ## # The following are the default conversions from Ruby objects into # low-level C++ objects. diff --git a/lib/v8/weak.rb b/lib/v8/weak.rb index 4614798..9903c73 100644 --- a/lib/v8/weak.rb +++ b/lib/v8/weak.rb @@ -39,7 +39,14 @@ module V8 end def []=(key, value) - @values[key] = V8::Weak::Ref.new(value) + V8::Weak::Ref.new(value).tap do |ref| + @values[key] = ref + ObjectSpace.define_finalizer(value, self.class.cleanup_entry(@values, key, ref)) + end + end + + def self.cleanup_entry(values, key, ref) + proc { values.delete(key) if values[key] == ref } end end end @@ -70,4 +77,4 @@ module V8 end end end -end \ No newline at end of file +end diff --git a/spec/mem/identity_mapping_spec.rb b/spec/mem/identity_mapping_spec.rb new file mode 100644 index 0000000..3bd23fa --- /dev/null +++ b/spec/mem/identity_mapping_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe "Identity Mapping Memory Usages" do + it "does not leak memory by virtue of maintaining an identity map" do + ## + # This verifies that the identity map which is holds on to the + # mappings between v8 objects and ruby objects does not leak its + # entries as originally reported by this ticket: + # https://github.com/cowboyd/therubyracer/pull/336 + # + # Without the patch, while the v8 object was not being leaked, the + # entry in the identity map was. This test injects 5000 objects + # into the v8 context 20 times over, which will generate 5000 + # unique proxies. All of these proxies should be immediately released. + + require 'memory_profiler' + + def rss + `ps -eo pid,rss | grep #{Process.pid} | awk '{print $2}'`.to_i + end + + cxt = V8::Context.new + Object.new.tap do |o| + cxt["object-#{o.object_id}"] = o + cxt["object-#{o.object_id}"] = nil + end + + # MemoryProfiler has a helper that runs the GC multiple times to make + # sure all objects that can be freed are freed. + MemoryProfiler::Helpers.full_gc + + start = rss + #puts "rss: #{rss} live objects #{GC.stat[:heap_live_slots]}" + + 20.times do + + 5000.times { |i| + Object.new.tap do |o| + cxt["object-#{o.object_id}"] = o + cxt["object-#{o.object_id}"] = nil + end + while cxt.isolate.native.IdleNotificationDeadline(0.1); end + } + MemoryProfiler::Helpers.full_gc + #puts "rss: #{rss} live objects #{GC.stat[:heap_live_slots]}" + + end + finish = rss + + expect(finish).to be <= start * 2.5 + end if RUBY_VERSION >= "2.1.0" +end diff --git a/spec/v8/context_spec.rb b/spec/v8/context_spec.rb index 3116d48..342f624 100644 --- a/spec/v8/context_spec.rb +++ b/spec/v8/context_spec.rb @@ -141,6 +141,10 @@ describe "V8::Context" do @cxt = V8::Context.new # @cxt['o'] = @instance end + it "can embed nil into a context" do + @cxt['nil'] = nil + expect(@cxt['nil']).to be_nil + end it "can embed a closure into a context and call it" do @cxt["say"] = lambda { |*args| args[-2] * args[-1] } diff --git a/therubyracer.gemspec b/therubyracer.gemspec index bf67f9b..9b3331e 100644 --- a/therubyracer.gemspec +++ b/therubyracer.gemspec @@ -19,4 +19,6 @@ Gem::Specification.new do |gem| gem.add_dependency 'ref' gem.add_dependency 'libv8', '~> 4.5.95.0' + + gem.add_development_dependency 'memory_profiler' end