Bug #7383

Use stricter cache check in load.c

Added by Yura Sokolov over 1 year ago. Updated over 1 year ago.

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

Description

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

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

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

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

Associated revisions

Revision 37808
Added by shirosaki over 1 year ago

Fix cache validity check of require

  • array.c (rbarysharedwithp): 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.
    [Bug #7383]

  • test/ruby/testrequire.rb
    (TestRequire#test
    requirewitharraypop,
    TestRequire#test
    requirewitharray_shift): add tests for above.

History

#1 Updated by Hiroshi Shirosaki over 1 year ago

Yura, thank you.
Indeed rbarysharedwithp 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?

#2 Updated by Yura Sokolov over 1 year ago

Hiroshi,

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

With regards,
Yura

2012/11/19 h.shirosaki (Hiroshi Shirosaki) h.shirosaki@gmail.com

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

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

Yura, thank you.
Indeed rbarysharedwithp 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: funnyfalcon (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]

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

This patch introduce rbarydupofp , 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/

#3 Updated by Hiroshi Shirosaki over 1 year ago

On Thu, Nov 22, 2012 at 6:09 PM, Юрий Соколов funny.falcon@gmail.com wrote:

Adding size check is enough in this case.
But rbarydupofp 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

#4 Updated by Anonymous over 1 year 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 (rbarysharedwithp): 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.
    [Bug #7383]

  • test/ruby/testrequire.rb
    (TestRequire#test
    requirewitharraypop,
    TestRequire#test
    requirewitharray_shift): add tests for above.

Also available in: Atom PDF