Bug #7383: Use stricter cache check in load.c

• Status: Closed
• Assignee: h.shirosaki (Hiroshi Shirosaki)
• Target version: 2.0.0
• ruby -v: ruby 2.0.0dev (2012-11-18 trunk 37708) [x86_64-linux]

[funny_falcon (Yura Sokolov)]

rb_ary_shared_with_p does not react when #shift or #pop is called on original array.

This patch introduce rb_ary_dup_of_p , which makes more adequate check for duplicate
of array.

https://github.com/ruby/ruby/pull/216
https://github.com/ruby/ruby/pull/216.patch

[h.shirosaki (Hiroshi Shirosaki)]

• File 0001-Fix-cache-validity-check-of-require.patch 0001-Fix-cache-validity-check-of-require.patch added

Yura, thank you.
Indeed rb_ary_shared_with_p does not react against #shift or #pop. Original array remains shared with the snapshot.
However, I think adding a check of array length would be sufficient for that. This fix has no performance impact, still O(1). I've attached a patch with tests.
Are there any reasons that such a strict check is needed?

[funny_falcon (Yura Sokolov)]

Hiroshi,

Adding size check is enough in this case.
But rb_ary_dup_of_p is more useful in the wild, I think.

With regards,
Yura

2012/11/19 h.shirosaki (Hiroshi Shirosaki) [e-mail]

Issue #7383 has been updated by h.shirosaki (Hiroshi Shirosaki).

File 0001-Fix-cache-validity-check-of-require.patch added

Yura, thank you.
Indeed rb_ary_shared_with_p does not react against #shift or #pop.
Original array remains shared with the snapshot.
However, I think adding a check of array length would be sufficient for
that. This fix has no performance impact, still O(1). I've attached a patch
with tests.
Are there any reasons that such a strict check is needed?

Bug #7383: Use stricter cache check in load.c
https://bugs.ruby-lang.org/issues/7383#change-33097

Author: funny_falcon (Yura Sokolov)
Status: Open
Priority: High
Assignee: h.shirosaki (Hiroshi Shirosaki)
Category: core
Target version: 2.0.0
ruby -v: ruby 2.0.0dev (2012-11-18 trunk 37708) [x86_64-linux]

rb_ary_shared_with_p does not react when #shift or #pop is called on
original array.

This patch introduce rb_ary_dup_of_p , which makes more adequate check for
duplicate
of array.

https://github.com/ruby/ruby/pull/216
https://github.com/ruby/ruby/pull/216.patch

--
http://bugs.ruby-lang.org/

[h.shirosaki (Hiroshi Shirosaki)]

On Thu, Nov 22, 2012 at 6:09 PM, Юрий Соколов [e-mail] wrote:

Adding size check is enough in this case.
But rb_ary_dup_of_p is more useful in the wild, I think.

OK. Then I'll apply my patch for now. If real issues were raised, I'll
consider your patch.

--
Hiroshi Shirosaki

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r37808.
Yura, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

Fix cache validity check of require

• array.c (rb_ary_shared_with_p): fix cache validity check.
  If #pop or #shift has been called against $: or $", the array will
  be still shared with the snapshot. We check array length for cache
  validity.
  [ruby-core:49518] [Bug #7383]

• test/ruby/test_require.rb
  (TestRequire#test_require_with_array_pop,
  TestRequire#test_require_with_array_shift): add tests for above.