socketry/timers

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

Closed this issue · 5 comments

Burgestrand commented

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 commented

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

#4

Burgestrand commented

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

Burgestrand commented

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

tarcieri commented

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

cremno commented

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.