Bug #17739: Array#sort! changes the order even if the receiver raises FrozenError in given block

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin20]
• Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN

[kachick (Kenichi Kamiya)]

I think this is a similar issue of https://bugs.ruby-lang.org/issues/17736

array = [1, 2, 3, 4, 5]
begin
  array.sort! do |a, b|
    array.freeze if a == 3
    1
  end
rescue => err
  p err #=> #<FrozenError: can't modify frozen Array: [5, 4, 3, 2, 1]>
end
p array #=> [5, 4, 3, 2, 1]

array = [1, 2, 3, 4, 5]
array.sort! do |a, b|
  break if a == 3
  1
end
p array #=> [3, 4, 2, 1, 5]

Array#sort! raises a FrozenError as expected, but the order is changed. I would expect the order is kept after frozen.

[chrisseaton (Chris Seaton)]

I think the point of #sort! is that it sorts in-place - so not creating a copy. If we want to be able to restore the unsorted array after an exception happens half-way through sorting, we'd need to create a copy to either swap back in after an exception, or to sort and then swap in on success. If we did either of those... we might as well not have #sort! at all and advise people to just use #sort, as you'd never really be 'sorting in place'.

[marcandre (Marc-Andre Lafortune)]

Is there a usecase for this?

[jeremyevans0 (Jeremy Evans)]

I think this is a bug, but it's not that Array#sort! should keep the initial order of the receiver, it's that it should keep the order at the point freeze is called, and it does not:

array = [1, 2, 3, 4, 5]
begin
  array.sort! do |a, b|
    array.freeze if a == 3
    p [array.frozen?, array]
    1
  end
rescue => err
  p err #=> #<FrozenError: can't modify frozen Array: [5, 4, 3, 2, 1]>
end

Output

[false, [1, 2, 3, 4, 5]]
[true, [1, 2, 3, 4, 5]]
[true, [5, 2, 3, 4, 1]]
[true, [5, 2, 3, 4, 1]]
[true, [5, 3, 2, 4, 1]]
[true, [5, 3, 2, 4, 1]]
#<FrozenError: can't modify frozen Array: [5, 3, 1, 4, 2]>

Notice that the array's order is changed after it has been frozen.

I've added a pull request to fix this: https://github.com/ruby/ruby/pull/4335

[matz (Yukihiro Matsumoto)]

Accepted. I have small concerns about performance (in microbenchmarks). But I don't think I have another choice.

Matz.

• Status changed from Open to Closed

Applied in changeset git|8b00bfb7c2c33827490c78a16c44b102cb0d724b.

---

Do not allow array modifications after freeze inside sort!

If freezing an array inside sort!, previously the array could be
modified after the freeze. This checks whether the receiver is
frozen after every yield and potential call to #> or #<,
preventing modifications if the receiver is frozen inside the
block or by the #> or #< call.

Fixes [Bug #17739]

Do not allow array modifications after freeze inside sort!

If freezing an array inside sort!, previously the array could be
modified after the freeze. This checks whether the receiver is
frozen after every yield and potential call to #> or #<,
preventing modifications if the receiver is frozen inside the
block or by the #> or #< call.

Fixes [Bug #17739]

[nobu (Nobuyoshi Nakada)]

Refined portability of test for [Bug #17739]

The order of arguments to callback of qsort is not defined.
That means a may not be 3 at all.