Bug #15479: Array#reject! modifies literal Array

• Status: Closed
• Assignee: tenderlovemaking (Aaron Patterson)
• Target version: -
• ruby -v: ruby 2.6.0p0 (2018-12-25 revision 66547) [x86_64-linux]
• Backport: 2.4: DONE, 2.5: DONE, 2.6: DONE

[Eregon (Benoit Daloze)]

This was found by running ruby/spec with -R2 (such as mspec -R2 -fs core/array/reject_spec.rb).
TravisCI log on https://travis-ci.org/ruby/spec/jobs/473175799#L539

Here is a simple reproducer.
MRI seems to modify the Array literal permanently:

3.times do
  a = [1, 2, 3, 4]
  puts "initial: #{a}"
  begin
    a.reject! do |x|
      case x
      when 2 then true
      when 3 then raise StandardError, 'Oops'
      else false
      end
    end
  rescue StandardError
  end

  puts "after:   #{a}"
end

prints

initial: [1, 2, 3, 4]
after:   [1, 3, 4]
initial: [1, 3, 4, 4]
after:   [1, 3, 4, 4]
initial: [1, 3, 4, 4]
after:   [1, 3, 4, 4]

2.5.3 behaves fine, but trunk is also affected.

[MSP-Greg (Greg L)]

Is the distinction really whether 2.6.0 & trunk only modify the array if reject! successfully enumerates all array members?

But if so, why does it modify the first loop? Strange behavior...

[k0kubun (Takashi Kokubun)]

• Assignee set to tenderlovemaking (Aaron Patterson)

@tenderlovemaking (Aaron Patterson) According to git bisect, https://github.com/ruby/ruby/commit/d46cd60f3cfb88f1591d6ce0f23e750d3833434f (Feature #15289) seems to have triggered this. Could you check it?

[mrkn (Kenta Murata)]

This is a smaller reproducible code.

mrkn-mbp15-late2016:bigdecimal mrkn$ cat t.rb
def foo
  a = [1, 2, 3, 4]
  a.reject! {|x|
    next true if x == 2
    return a if x == 3
    false
  }
end

p foo
p foo
mrkn-mbp15-late2016:bigdecimal mrkn$ ruby -v t.rb
ruby 2.6.0p0 (2018-12-25 revision 66547) [x86_64-darwin18]
[1, 3, 4]
[1, 3, 4, 4]

[k0kubun (Takashi Kokubun)]

Oops, I copy-pasted the ticket number in the commit message, but actually it should have been Feature #15349.

[k0kubun (Takashi Kokubun)]

• Related to Feature #15349: Use a shared array for the `duparray` instruction added

[mrkn (Kenta Murata)]

• Status changed from Open to Assigned

[k0kubun (Takashi Kokubun)]

• Status changed from Assigned to Closed

Applied in changeset trunk|r66632.

---

erb.rb: increase warn level only when non-zero safe_level

is given.

This is merging Eric's patch in [Bug #15479] to Ruby 2.6's behavior in r66631.

[k0kubun (Takashi Kokubun)]

• Status changed from Closed to Assigned

oops, closed this ticket by a wrong ticket number in my commit... reopening.

[tenderlovemaking (Aaron Patterson)]

It seems like this is also a bug in Ruby 2.5 and Ruby 2.4. If you modify the test program a little bit like this:

def foo
  b = [1, 2, 3, 4]
  3.times do
    a = b.dup
    puts "initial: #{a}"
    begin
      a.reject! do |x|
        case x
        when 2 then true
        when 3 then raise StandardError, 'Oops'
        else false
        end
      end
    rescue StandardError
    end

    puts "after:   #{a}"
  end
end

foo

It will fail on 2.4.X, 2.5.X, and 2.6. It seems like "reject!" isn't unsharing the array. I will make a patch to fix this.

[tenderlovemaking (Aaron Patterson)]

• Status changed from Assigned to Closed

Applied in changeset trunk|r66756.

---

Mark array as "going to be modified" in Array#reject!

Before this patch, if reject! is called on a shared array it can
mutate the shared array rather than a copy. This patch marks the array
as "going to be modified" so that the shared source array isn't
mutated.

[Bug #15479] [ruby-core:90781]

[tenderlovemaking (Aaron Patterson)]

• Backport changed from 2.4: DONTNEED, 2.5: DONTNEED, 2.6: REQUIRED to 2.4: REQUIRED, 2.5: REQUIRED, 2.6: REQUIRED

r66756 should be backported, so I updated the backport field.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.4: REQUIRED, 2.5: REQUIRED, 2.6: REQUIRED to 2.4: REQUIRED, 2.5: DONE, 2.6: REQUIRED

ruby_2_5 r66784 merged revision(s) 66756.

[naruse (Yui NARUSE)]

• Backport changed from 2.4: REQUIRED, 2.5: DONE, 2.6: REQUIRED to 2.4: REQUIRED, 2.5: DONE, 2.6: DONE

ruby_2_6 r66847 merged revision(s) 66756.

[usa (Usaku NAKAMURA)]

• Backport changed from 2.4: REQUIRED, 2.5: DONE, 2.6: DONE to 2.4: DONE, 2.5: DONE, 2.6: DONE

ruby_2_4 r66966 merged revision(s) 66756.