Skip to content

Commit

Permalink
Use iteration count for final size in map. Fixes #3155.
Browse files Browse the repository at this point in the history
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.
headius committed Jul 21, 2015
1 parent a8ee690 commit 59fe0bf
Showing 2 changed files with 13 additions and 2 deletions.
6 changes: 4 additions & 2 deletions core/src/main/java/org/jruby/RubyArray.java
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 9 additions & 0 deletions spec/regression/GH-3155_array_map_with_mutation_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 59fe0bf

Please sign in to comment.