Race condition in JRuby when Active Support is overriding Kernel.require

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.1.6
  • Fix Version/s: JRuby 1.2
  • Component/s: Application Error
  • Labels:
    None
  • Environment:
    Solaris 10 (SPARC with 4 CPU), OSX 10.5 (Intel with 2 CPU). The problem probably occurs on any platform capable for running JRuby.

Description

A race condition occurs in JRuby (Cf. call stack below) when ActiveSupport overrides kernel.require and that require 'mylib' is being called whitin a thread (Cf. attached sample to reproduce the issue).

This error is especially a problem with rexml since it tries to require 'rexml/encodings/UTF-8.rb' (in $ruby_home/lib/ruby/1.8/rexml/encoding.rb (line 48)) everytime the parser is called.

Here is a snapshot of the callstack:

Exception in thread "Thread-2608" java.lang.ArrayIndexOutOfBoundsException
at org.jruby.RubyArray.fillNil(RubyArray.java:383)
at org.jruby.RubyArray.splice(RubyArray.java:871)
at org.jruby.RubyArray.concat(RubyArray.java:1425)
at org.jruby.RubyArray$i_method_1_0$RUBYINVOKER$concat.call(org/jruby/RubyArray$i_method_1_0$RUBYINVOKER$concat.gen)
at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:114)
at ruby.jit.ruby.Users.jdmorani.Downloads.jruby_minus_1_dot_1_dot_6.lib.ruby.gems.$1_dot_8.gems.activesupport_minus_2_dot_2_dot_2.lib.active_support.dependencies.new_constants_in264324716_1186545073.ensure_1$RUBY$_ensure__0(dependencies.rb:517)
at ruby.jit.ruby.Users.jdmorani.Downloads.jruby_minus_1_dot_1_dot_6.lib.ruby.gems.$1_dot_8.gems.activesupport_minus_2_dot_2_dot_2.lib.active_support.dependencies.new_constants_in264324716_1186545073._file_(dependencies.rb)
at org.jruby.ast.executable.AbstractScript._file_(AbstractScript.java:31)
at org.jruby.internal.runtime.methods.DefaultMethod.call(DefaultMethod.java:258)
at org.jruby.runtime.callsite.CachingCallSite.callIter(CachingCallSite.java:139)
at ruby.jit.ruby.Users.jdmorani.Downloads.jruby_minus_1_dot_1_dot_6.lib.ruby.gems.$1_dot_8.gems.activesupport_minus_2_dot_2_dot_2.lib.active_support.dependencies.require1129611392_1186545073.rescue_1$RUBY$_rescue__0(dependencies.rb:153)
at ruby.jit.ruby.Users.jdmorani.Downloads.jruby_minus_1_dot_1_dot_6.lib.ruby.gems.$1_dot_8.gems.activesupport_minus_2_dot_2_dot_2.lib.active_support.dependencies.require1129611392_1186545073._file_(dependencies.rb)
at org.jruby.ast.executable.AbstractScript._file_(AbstractScript.java:31)
at org.jruby.internal.runtime.methods.DefaultMethod.call(DefaultMethod.java:237)
at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:114)
at $dot.test.block_1$RUBY$_block_(test.rb:19)
at $dot.testBlockCallback$block_1$RUBY$_block_xx1.call(Unknown Source)
at org.jruby.runtime.CompiledBlock.yield(CompiledBlock.java:120)
at org.jruby.runtime.BlockBody.call(BlockBody.java:64)
at org.jruby.runtime.BlockBody.call(BlockBody.java:70)
at org.jruby.runtime.Block.call(Block.java:116)
at org.jruby.RubyProc.call(RubyProc.java:205)
at org.jruby.RubyProc.call(RubyProc.java:187)
at org.jruby.internal.runtime.RubyRunnable.run(RubyRunnable.java:90)
at java.lang.Thread.run(Thread.java:637)

  1. File
    test.rb
    17/Jan/09 12:08 PM
    3 kB
    Jean-Dominique Morani

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Charles Oliver Nutter added a comment -

This seems to be something else. The code being called is in Array#concat, as part of ActiveSupport's logic for building a list of constants when a file is required. My guess is that the concurrent requires are triggering this, but that it's a concurrency issue within ActiveSupport itself. In this case, if it's allowing concurrent requires to mutate the same collection without a lock, then it will never be safe even under MRI...the two threads will likely overwrite each others results.

This may be a bug in ActiveSupport itself. I will look at the Array method (concat) to see if we can raise a better error, but this does appear to be a concurrency issue within ActiveSupport.

Show
Charles Oliver Nutter added a comment - This seems to be something else. The code being called is in Array#concat, as part of ActiveSupport's logic for building a list of constants when a file is required. My guess is that the concurrent requires are triggering this, but that it's a concurrency issue within ActiveSupport itself. In this case, if it's allowing concurrent requires to mutate the same collection without a lock, then it will never be safe even under MRI...the two threads will likely overwrite each others results. This may be a bug in ActiveSupport itself. I will look at the Array method (concat) to see if we can raise a better error, but this does appear to be a concurrency issue within ActiveSupport.
Hide
Permalink
Charles Oliver Nutter added a comment -

I looked at the code and I think we can do little more than improve the error. The problem is that the Array is under mutation from multiple threads at the same time, which is not safe in JRuby. Rails has in general wrapped such mutations with a mutext, and I imagine this is just a case that was missed. There's also an argument to be made that Rails development mode is not intended to be used with multiple threads, and I believe that's the mode you're running in here. So I would recommend you either do not use threads with this code or do not run in development mode; and it would probably be a good idea to file this as an issue with Rails core.

I am committing a small improvement that at least throws a normal Ruby exception (ConcurrencyError) instead of a Java exception, but it doesn't change the fact that this is an unsafe concurrent modification happening in ActiveSupport.

Show
Charles Oliver Nutter added a comment - I looked at the code and I think we can do little more than improve the error. The problem is that the Array is under mutation from multiple threads at the same time, which is not safe in JRuby. Rails has in general wrapped such mutations with a mutext, and I imagine this is just a case that was missed. There's also an argument to be made that Rails development mode is not intended to be used with multiple threads, and I believe that's the mode you're running in here. So I would recommend you either do not use threads with this code or do not run in development mode; and it would probably be a good idea to file this as an issue with Rails core. I am committing a small improvement that at least throws a normal Ruby exception (ConcurrencyError) instead of a Java exception, but it doesn't change the fact that this is an unsafe concurrent modification happening in ActiveSupport.
Hide
Permalink
Charles Oliver Nutter added a comment -

I created a patch for Rails, since I believe this is a Rails issue. I reported the bug and attached the patch here:

http://rails.lighthouseapp.com/projects/8994/tickets/1816-constant_watch_stack-must-be-protected-by-a-mutex#ticket-1816-1

Show
Charles Oliver Nutter added a comment - I created a patch for Rails, since I believe this is a Rails issue. I reported the bug and attached the patch here: http://rails.lighthouseapp.com/projects/8994/tickets/1816-constant_watch_stack-must-be-protected-by-a-mutex#ticket-1816-1
Hide
Permalink
Charles Oliver Nutter added a comment -

The patch I submitted to Rails has been merged into master and will be in Rails 2.3. Have fun!

Show
Charles Oliver Nutter added a comment - The patch I submitted to Rails has been merged into master and will be in Rails 2.3. Have fun!

People

  • Assignee:
    Charles Oliver Nutter
    Reporter:
    Jean-Dominique Morani
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: