ConcurrentModificationException in subclasses map

[headius]

Discovered by @kalenp while attempting to deploy 9.4.11.0 to their test environments.

It appears at least one call path into the new subclasses collections (Map.remove) was not properly synchronized, which can lead to ConcurrentModificationException under concurrent load and modification of the class hierarchy.

The following test is sufficient to reproduce the exception most of the time. There are many different types of class hierarchy modification here and several attempts to force traversal of all subclasses, so I'm unsure which operations specifically can trigger this error. The test is intended to try everything under reasonably high concurrent load.

require 'test/unit'

class TestClassSubclasses < Test::Unit::TestCase
  def test_concurrent_modification
    c = Class.new
    c2 = Class.new(c)
    go = false

    thread_count = 100
    threads = thread_count.times.map {
      Thread.new {
        Thread.pass until go
        c3 = Class.new(c2)
        o = c3.new
        c.class_eval {
          def foo; end
        }
        class << o
          extend Enumerable
          prepend Comparable
        end
        c.class_eval {
          def foo; end
        }
        c3.class_eval {
          include Enumerable
          prepend Comparable
        }
        c.class_eval {
          def foo; end
        }
        :success
      }
    }

    go = true
    assert_equal(threads.map(&:value), [:success] * thread_count)
  end
end

A high percentage of runs of this test will produce output similar to the following:

$ jruby test/jruby/test_class_subclasses.rb
Loaded suite test/jruby/test_class_subclasses
Started
java.util.ConcurrentModificationException
	at java.base/java.util.WeakHashMap.forEach(WeakHashMap.java:1037)
	at org.jruby.dist/org.jruby.RubyClass$SubclassesWeakMap.forEach(RubyClass.java:1191)
	at org.jruby.dist/org.jruby.RubyClass.invalidateCacheDescendants(RubyClass.java:1255)
	at org.jruby.dist/org.jruby.RubyClass.lambda$invalidateCacheDescendants$5(RubyClass.java:1255)
	at java.base/java.util.WeakHashMap.forEach(WeakHashMap.java:java.util.ConcurrentModificationException
	at java.base/java.util.WeakHashMap.forEach(WeakHashMap.java:1037)
	at org.jruby.dist/org.jruby.RubyClass$SubclassesWeakMap.forEach(RubyClass.java:1191)
	at org.jruby.dist/org.jruby.RubyClass.invalidateCacheDescendants(RubyClass.java:1255)
	at org.jruby.dist/org.jruby.RubyClass.lambda$invalidateCacheDescendants$5(RubyClass.java:1255)
...many more lines of similar output
	at org.jruby.dist/org.jruby.runtime.CompiledIRBlockBody.callDirect(CompiledIRBlockBody.java:141)
	at org.jruby.dist/org.jruby.runtime.IRBlockBody.call(IRBlockBody.java:64)
	at org.jruby.dist/org.jruby.runtime.IRBlockBody.call(IRBlockBody.java:58)
	at org.jruby.dist/org.jruby.runtime.Block.call(Block.java:144)
	at org.jruby.dist/org.jruby.RubyProc.call(RubyProc.java:354)
	at org.jruby.dist/org.jruby.internal.runtime.RubyRunnable.run(RubyRunnable.java:111)
	at java.base/java.lang.Thread.run(Thread.java:1583)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1 tests, 0 assertions, 0 failures, 1 errors, 0 pendings, 0 omissions, 0 notifications
0% passed
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
17.62 tests/s, 0.00 assertions/s

I have a fix in progress.

[headius]

The likely candidate is calls to Map.remove against the subclasses map. Unlike put and forEach, this method was not overridden in our SubclassesWeakMap to properly use a read/write lock.

[kares]

@kalenp was there a Rails specific trace for this bug, that you could maybe share (for reference)?

[kalenp]

We're not running Rails, but we would see it in our CI with rake_test_loader. A sample backtrace:

LoadError: load error: test/test_helper_base -- java.util.ConcurrentModificationException: null
Did you mean?  test/test_helper_base
  require at org/jruby/RubyKernel.java:1187
  require at /nix/store/aqh591556wi2gdjrymzhdp7hv7z41jma-jruby-9.4.11.0/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:37
   <main> at test/test_helper.rb:3
  require at org/jruby/RubyKernel.java:1187
  require at /nix/store/aqh591556wi2gdjrymzhdp7hv7z41jma-jruby-9.4.11.0/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:37
   <main> at test/functional/test_example_controller.rb:3
  require at org/jruby/RubyKernel.java:1187
  require at /nix/store/aqh591556wi2gdjrymzhdp7hv7z41jma-jruby-9.4.11.0/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:37
   <main> at /nix/store/aqh591556wi2gdjrymzhdp7hv7z41jma-jruby-9.4.11.0/lib/ruby/gems/shared/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:21
   select at org/jruby/RubyArray.java:2910
   <main> at /nix/store/aqh591556wi2gdjrymzhdp7hv7z41jma-jruby-9.4.11.0/lib/ruby/gems/shared/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6