Project

General

Profile

Actions

Bug #7383

closed

Use stricter cache check in load.c

Added by funny_falcon (Yura Sokolov) over 11 years ago. Updated over 11 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 2.0.0dev (2012-11-18 trunk 37708) [x86_64-linux]
Backport:
[ruby-core:49518]

Description

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


Files

0001-Fix-cache-validity-check-of-require.patch (2.57 KB) 0001-Fix-cache-validity-check-of-require.patch h.shirosaki (Hiroshi Shirosaki), 11/19/2012 10:55 PM

Updated by h.shirosaki (Hiroshi Shirosaki) over 11 years ago

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?

Updated by funny_falcon (Yura Sokolov) over 11 years ago

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)

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/

Updated by h.shirosaki (Hiroshi Shirosaki) over 11 years ago

On Thu, Nov 22, 2012 at 6:09 PM, Юрий Соколов 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

Actions #4

Updated by Anonymous over 11 years ago

  • 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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0