Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First creation of SortedSet is not thread-safe, sometimes causing load errors in Celluloid #20

Closed
Burgestrand opened this issue Dec 3, 2013 · 5 comments

Comments

@Burgestrand
Copy link

We have this lovely method: https://github.com/ruby/ruby/blob/ruby_2_0_0/lib/set.rb#L532

When initializing a sorted set for the first time, it will define the implementation of SortedSet, and this is not thread-safe: https://github.com/ruby/ruby/blob/ruby_2_0_0/lib/set.rb#L627

Due to this, I’ve sometimes seen an error:

org.jruby.exceptions.RaiseException: (NameError) method 'old_init' not defined in SortedSet
  at org.jruby.RubyModule.remove_method(org/jruby/RubyModule.java:2340) [jruby.jar:]
  at RUBY.setup(/Users/dev/.rvm/rubies/jruby-1.7.8/lib/ruby/1.9/set.rb:609)  
  at org.jruby.RubyModule.module_eval(org/jruby/RubyModule.java:2300) [jruby.jar:]
  at RUBY.setup(/Users/dev/.rvm/rubies/jruby-1.7.8/lib/ruby/1.9/set.rb:607)  
  at RUBY.initialize(/Users/dev/.rvm/rubies/jruby-1.7.8/lib/ruby/1.9/set.rb:617)  
  at RUBY.initialize(/Users/dev/.rvm/gems/jruby-1.7.8/gems/timers-1.1.0/lib/timers.rb:12)  
  at RUBY.initialize(/Users/dev/.rvm/gems/jruby-1.7.8/gems/celluloid-0.15.2/lib/celluloid/receivers.rb:9)  
  at RUBY.initialize(/Users/dev/.rvm/gems/jruby-1.7.8/gems/celluloid-0.15.2/lib/celluloid/actor.rb:149)  
  at RUBY.new(/Users/dev/.rvm/gems/jruby-1.7.8/gems/celluloid-0.15.2/lib/celluloid.rb:188)  
  at RUBY.supervise(/Users/dev/.rvm/gems/jruby-1.7.8/gems/celluloid-0.15.2/lib/celluloid/supervisor.rb:10)  
  at RUBY.supervise(/Users/dev/.rvm/gems/jruby-1.7.8/gems/celluloid-0.15.2/lib/celluloid.rb:208)  
  at RUBY.start(/Users/dev/Projects/jupiter/app/services/ping_service.rb:25)

Here’s a fix we’ve used to get around this, but perhaps people using the timers gem should not need to use this themselves:

require "set"
SortedSet.new

Additionally, if the gem rbtree is not available, SortedSet uses an implementation which is kind of insane. If timers uses SortedSet, it might be nice to have a gem dependency on rbtree. One might want to bench the performance in comparison to the insane implementation first, though.

@tarcieri
Copy link
Contributor

tarcieri commented Dec 3, 2013

Last I benchmarked it, timers was actually slower with rbtree. See some of the previous discussion here:

#4

@Burgestrand
Copy link
Author

I suspected as much; glad to have that out of the way. The thread-safety issue still remains though.

@Burgestrand
Copy link
Author

Then again, ideally this would be fixed in ruby stdlib. ;)

@tarcieri
Copy link
Contributor

tarcieri commented Dec 3, 2013

The workaround is a bit gross but I wouldn't mind it if it fixes the problem. Would you mind sending a PR?

@cremno
Copy link

cremno commented Dec 4, 2013

ruby/ruby#451 and https://bugs.ruby-lang.org/issues/9121 are somewhat related. But both discussions don't mention thread safety, only issues with rbtree and that terrible method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants