Non adapting end of iteration on insert during iteration - difference between Ruby and JRuby

[Liberatys]

Issue:

There seems to be a difference in how JRuby and Ruby handle insertion of elements into an array that is being iterated over.

The version of JRuby doesn't seem to matter. Therefore I suspect a fairly old incompatibility

Versions:

• JRuby: jruby 9.2.9.0
• Ruby: 2.6.5
• OS: Darwin RMBP 19.6.0 Darwin Kernel Version 19.6.0: Sun Jul 5 00:43:10 PDT 2020

Test:

rules = [1, 2, 3, 4, 5]
rules.delete_if.with_index do |rule, index|
  p rule
  rules.insert(index + 1, 2) if rule == 1
  true
end

Running the test

Execute the code above with both JRuby and Ruby.
E.g. ruby file_name.rb

Expected Behavior
Ruby

1
2
2
3
4	
5

Actual Behavior
JRuby

1
2
2
3
4

My guess

I guess that JRuby sets the max_index/end of an iteration at the beginning of an iteration and Ruby adapts that end if an insert occurs.

I do not want to impose this behaviour on JRuby if it is not how it's intended to be. But I expected it to behave like Ruby.

[Liberatys]

Does not arise with

rules = [1, 2, 3, 4, 5]
rules.each_with_index do |rule, index|
	p rule
    rules.insert(index + 1, 2) if rule == 1
end

[coorasse]

This issue is what currently makes CanCanCan tests fail on JRuby.

[schmijos]

While it's for sure weird that Ruby and JRuby behave differently, I'd ask myself what to expect when altering a collection I'm iterating over. Even more because your example mixes indexes and iterators.

[Liberatys]

The issue also exists in iteration without index ^^

rules = [1, 2, 3, 4, 5]
rules.delete_if do |rule|
  p rule
  rules.insert(1, 2) if rule == 1
  true
end

And I expect it to behave the same way that ruby does.

I am well aware that other languages such as java permit such behaviour which is why I removed it from the codebase I found it in... but still...
JRuby should not behave in a different way than Ruby :D
Or if they behave in a different way they should not use the term "compatible".

[headius]

The official position of matz has always been that modification during iteration is an unspecified behavior, so I am not surprised to see a difference here. That said, we will make a best effort to match behavior here.

[headius]

The fix is fairly straightforward and follows similar changes in other iteration methods:

diff --git a/core/src/main/java/org/jruby/RubyArray.java b/core/src/main/java/org/jruby/RubyArray.java
index 4b47dc522a..244ec9a07d 100644
--- a/core/src/main/java/org/jruby/RubyArray.java
+++ b/core/src/main/java/org/jruby/RubyArray.java
@@ -2828,7 +2828,7 @@ public class RubyArray<T extends IRubyObject> extends RubyObject implements List
         int len0 = 0, len1 = 0;
         try {
             int i1, i2;
-            for (i1 = i2 = 0; i1 < len; len0 = ++i1) {
+            for (i1 = i2 = 0; i1 < realLength; len0 = ++i1) {
                 final IRubyObject[] values = this.values;
                 // Do not coarsen the "safe" check, since it will misinterpret AIOOBE from the yield
                 // See JRUBY-5434

By memoizing the length, we don't provide any means for it to be updated during the loop, other than via our own deletions. This may be less safe for concurrency, since it will also see length changes by other threads and potentially try to walk off the end of the array.

I will compare with CRuby to confirm this is how they handle the issue.

[Liberatys]

Thank you for looking into it ^^ I agree that inserting into an array that is being iterated over is far from good practice. But it appears to be used in some cases and therefore I found it to be something I ought to report.

[headius]

This does appear to be how the equivalent code in CRuby is implemented. I will push this change to jruby-9.2 branch and it will be in 9.2.14 (if we do such a release) and 9.3.

However I strongly discourage depending on this behavior, and it would be safest for you to eliminate such uses in your apps and libraries.

This behavior is not tested by any of the existing compatibility suites – it's not in ruby/spec, not in CRuby's own tests, not in the additional tests JRuby has – because it is considered unspecified behavior. Behavior across Ruby implementations varies:

• JRuby and Opal: end offset is wrong, last element lost
• TruffleRuby (and probably Rubinius): raises array out of bounds error