Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

always lock v8 when accessing it. Never lock from Ruby GC

  • Loading branch information...
commit d627fbdbab8d7939709077b6e43cf954908a0c57 1 parent e48ee9e
@cowboyd cowboyd authored
View
1  ext/v8/v8.cpp
@@ -25,6 +25,7 @@ extern "C" {
extern "C" {
void Init_v8() {
+ v8::Locker locker;
rr_init_handle();
rr_init_context();
rr_init_value();
View
29 ext/v8/v8_handle.cpp
@@ -10,13 +10,11 @@ v8_handle::v8_handle(Handle<void> handle) : handle(Persistent<void>::New(handle)
this->dead = false;
}
-v8_handle::~v8_handle() {
- handle.Dispose();
- handle.Clear();
- dead = true;
-}
+v8_handle::~v8_handle() {}
namespace {
+ VALUE handle_queue;
+
void v8_handle_mark(v8_handle* handle) {
rb_gc_mark(handle->weakref_callback);
rb_gc_mark(handle->weakref_callback_parameters);
@@ -26,6 +24,21 @@ namespace {
delete handle;
}
+ void v8_handle_enqueue(v8_handle* handle) {
+ handle->dead = true;
+ VALUE zombie = Data_Wrap_Struct(rr_v8_handle_class(), 0, v8_handle_free, handle);
+ rb_ary_unshift(handle_queue, zombie);
+ }
+
+ void v8_handle_dequeue(GCType type, GCCallbackFlags flags) {
+ for (VALUE handle = rb_ary_pop(handle_queue); RTEST(handle); handle = rb_ary_pop(handle_queue)) {
+ v8_handle* dead = NULL;
+ Data_Get_Struct(handle, struct v8_handle, dead);
+ dead->handle.Dispose();
+ dead->handle.Clear();
+ }
+ }
+
VALUE New(VALUE self, VALUE handle) {
if (RTEST(handle)) {
Persistent<void> that = rr_v8_handle<void>(handle);
@@ -101,11 +114,15 @@ void rr_init_handle() {
rr_define_method(HandleClass, "ClearWeak", ClearWeak, 0);
rr_define_method(HandleClass, "IsNearDeath", IsNearDeath, 0);
rr_define_method(HandleClass, "IsWeak", IsWeak, 0);
+
+ handle_queue = rb_ary_new();
+ rb_gc_register_address(&handle_queue);
+ V8::AddGCPrologueCallback(v8_handle_dequeue);
}
VALUE rr_v8_handle_new(VALUE klass, v8::Handle<void> handle) {
v8_handle* new_handle = new v8_handle(handle);
- return Data_Wrap_Struct(klass, v8_handle_mark, v8_handle_free, new_handle);
+ return Data_Wrap_Struct(klass, v8_handle_mark, v8_handle_enqueue, new_handle);
}
VALUE rr_v8_handle_class() {
View
7 ext/v8/v8_locker.cpp
@@ -66,8 +66,8 @@ namespace {
* For details on V8 locking semantics, see the locking {API http://izs.me/v8-docs/classv8_1_1Unlocker.html}
* @return [V8::C::Unocker] the new locker
*/
- VALUE New(VALUE UnockerClass) {
- Unlocker unlocker = new Unlocker();
+ VALUE New(VALUE UnlockerClass) {
+ Unlocker* unlocker = new Unlocker();
return Data_Wrap_Struct(UnlockerClass, 0, 0, (void*)unlocker);
}
@@ -80,7 +80,7 @@ namespace {
*/
VALUE Delete(VALUE self) {
Unlocker* unlocker;
- Data_Get_Struct(self, class Locker, locker);
+ Data_Get_Struct(self, class Unlocker, unlocker);
delete unlocker;
}
}
@@ -126,7 +126,6 @@ namespace {
}
void rr_init_v8_locker() {
- VALUE V8 = rb_define_module("V8");
VALUE LockerClass = rr_define_class("Locker");
VALUE UnlockerClass = rr_define_class("Unlocker");
rr_define_singleton_method(LockerClass, "new", Lock::New, 0);
View
1  lib/v8.rb
@@ -4,6 +4,7 @@
module V8
require 'v8/version'
require 'v8/v8' #native glue
+ require 'v8/c/locker'
require 'v8/portal'
require 'v8/portal/caller'
require 'v8/portal/proxies'
View
12 lib/v8/c/locker.rb
@@ -0,0 +1,12 @@
+module V8
+ module C
+ # Shim to support the old style locking syntax. We don't currently
+ # deprecate this because it might make a comeback at some point.
+ def Locker
+ lock = Locker.new
+ yield
+ ensure
+ lock.delete
+ end
+ end
+end
View
105 lib/v8/context.rb
@@ -5,51 +5,58 @@ class Context
attr_reader :native, :scope, :access
def initialize(opts = {})
- @access = Access.new
- @to = Portal.new(self, @access)
- with = opts[:with]
- constructor = nil
- template = if with
- constructor = @to.templates.to_constructor(with.class)
- constructor.disable()
- constructor.template.InstanceTemplate()
- else
- C::ObjectTemplate::New()
- end
- @native = opts[:with] ? C::Context::New(template) : C::Context::New()
- @native.enter do
- @global = @native.Global()
- @to.proxies.register_javascript_proxy @global, :for => with if with
- constructor.enable() if constructor
- @scope = @to.rb(@global)
- @global.SetHiddenValue(C::String::NewSymbol("TheRubyRacer::RubyContext"), C::External::New(self))
+ lock do
+ @access = Access.new
+ @to = Portal.new(self, @access)
+ with = opts[:with]
+ constructor = nil
+ template = if with
+ constructor = @to.templates.to_constructor(with.class)
+ constructor.disable()
+ constructor.template.InstanceTemplate()
+ else
+ C::ObjectTemplate::New()
+ end
+ @native = opts[:with] ? C::Context::New(template) : C::Context::New()
+ @native.enter do
+ @global = @native.Global()
+ @to.proxies.register_javascript_proxy @global, :for => with if with
+ constructor.enable() if constructor
+ @scope = @to.rb(@global)
+ @global.SetHiddenValue(C::String::NewSymbol("TheRubyRacer::RubyContext"), C::External::New(self))
+ end
+ yield(self) if block_given?
end
- yield(self) if block_given?
end
def eval(javascript, filename = "<eval>", line = 1)
- if IO === javascript || StringIO === javascript
- javascript = javascript.read()
- end
- err = nil
- value = nil
- C::TryCatch.try do |try|
- @native.enter do
- script = C::Script::Compile(@to.v8(javascript.to_s), @to.v8(filename.to_s))
- if try.HasCaught()
- err = JSError.new(try, @to)
- else
- result = script.Run()
+ lock do
+ if IO === javascript || StringIO === javascript
+ javascript = javascript.read()
+ end
+ err = nil
+ value = nil
+ C::TryCatch.try do |try|
+ @native.enter do
+ script = C::Script::Compile(@to.v8(javascript.to_s), @to.v8(filename.to_s))
if try.HasCaught()
err = JSError.new(try, @to)
else
- value = @to.rb(result)
+ result = script.Run()
+ if try.HasCaught()
+ err = JSError.new(try, @to)
+ else
+ value = @to.rb(result)
+ end
end
end
end
+ if err
+ raise err
+ else
+ value
+ end
end
- raise err if err
- return value
end
def load(filename)
@@ -75,22 +82,36 @@ def self.stack(limit = 99)
[]
end
end
+
+ private
+
+ def lock
+ lock = V8::C::Locker.new
+ yield
+ ensure
+ lock.delete
+ end
end
module C
class Context
def enter
- if block_given?
- if IsEntered()
- yield(self)
- else
- Enter()
- begin
+ begin
+ lock = Locker.new
+ if block_given?
+ if IsEntered()
yield(self)
- ensure
- Exit()
+ else
+ Enter()
+ begin
+ yield(self)
+ ensure
+ Exit()
+ end
end
end
+ ensure
+ lock.delete
end
end
end
View
8 lib/v8/function.rb
@@ -4,8 +4,8 @@ class Function < V8::Object
def methodcall(thisObject, *args)
err = nil
return_value = nil
- C::TryCatch.try do |try|
- @portal.open do |to|
+ @portal.open do |to|
+ C::TryCatch.try do |try|
this = to.v8(thisObject)
return_value = to.rb(@native.Call(this, to.v8(args)))
err = JSError.new(try, to) if try.HasCaught()
@@ -16,7 +16,9 @@ def methodcall(thisObject, *args)
end
def call(*args)
- self.methodcall(@portal.context.native.Global(), *args)
+ @portal.open do
+ self.methodcall(@portal.context.native.Global(), *args)
+ end
end
def new(*args)
View
7 spec/ext/cxt_spec.rb
@@ -5,6 +5,13 @@
describe C::Context do
+ before do
+ @locker = C::Locker.new
+ end
+ after do
+ @locker.delete
+ end
+
it "should not have a current context if no context is open" do
C::Context::GetEntered().should be_nil
end
View
3  spec/ext/ext_spec_helper.rb
@@ -4,12 +4,13 @@ module V8::ExtSpec
def self.included(object)
object.class_eval do
before do
+ @lock = c::Locker.new
@cxt = c::Context::New()
@cxt.Enter()
end
after do
@cxt.Exit()
- @cxt.Dispose()
+ @lock.delete
end
end
end
View
4 spec/ext/try_catch_spec.rb
@@ -3,9 +3,7 @@
include V8
describe C::TryCatch do
-
- before {@cxt = C::Context::New()}
- after {@cxt.Dispose()}
+ include V8::ExtSpec
it "does not allow instance creation by default" do
lambda {
View
2  specthread/spec_helper.rb
@@ -0,0 +1,2 @@
+require Pathname(__FILE__).dirname.join('../spec/spec_helper')
+
View
13 specthread/threading_spec.rb
@@ -0,0 +1,13 @@
+
+require 'spec_helper'
+
+describe "using v8 from multiple threads" do
+
+ it "is possible" do
+ Thread.new do
+ require 'v8'
+ V8::Context.new
+ end.join
+ V8::Context.new
+ end
+end
Please sign in to comment.
Something went wrong with that request. Please try again.