Use iteration count for final size in map. Fixes #3155.

When the block passed to map makes modifications to the array under iteration, we may prematurely finish the map loop due to the size changing. However our logic for creating the mapped array assumed the new array's size would always be the same as the original array's size, leading to an array with null elements. This fix uses the iteration count as the final size, so we at least know how many elements in the new array were populated. Note that this behavior is officially undefined; modifying the array while performing internal iteration can cause peculiar effects across runtimes and potentially across the different versions of the same runtime. We add a regression spec here to at least make sure we don't produce an invalid array.
jruby · Jul 21, 2015 · 59fe0bf · 59fe0bf
1 parent a8ee690
commit 59fe0bf
Showing 2 changed files with 13 additions and 2 deletions.
diff --git a/core/src/main/java/org/jruby/RubyArray.java b/core/src/main/java/org/jruby/RubyArray.java
@@ -2393,13 +2393,15 @@ public IRubyObject collect(ThreadContext context, Block block) {
 
         IRubyObject[] arr = new IRubyObject[realLength];
 
-        for (int i = 0; i < realLength; i++) {
+        int i;
+        for (i = 0; i < realLength; i++) {
             // Do not coarsen the "safe" check, since it will misinterpret AIOOBE from the yield
             // See JRUBY-5434
             arr[i] = block.yield(context, safeArrayRef(values, i + begin));
         }
 
-        return new RubyArray(runtime, arr);
+        // use iteration count as new size in case something was deleted along the way
+        return new RubyArray(runtime, arr, 0, i);
     }
 
     @JRubyMethod(name = {"collect"}, compat = RUBY1_9)

diff --git a/spec/regression/GH-3155_array_map_with_mutation_spec.rb b/spec/regression/GH-3155_array_map_with_mutation_spec.rb
@@ -0,0 +1,9 @@
+describe "Array#map with delete inside the loop" do
+  it "must not produce invalid array contents" do
+    array = [1, 2, 3]
+    array2 = array.map { |v| array.delete(v); v + 1 }
+
+    expect(array2.compact).to eq array2
+    expect(array2.inspect).to eq "[2, 4]"
+  end
+end