Project

General

Profile

Bug #7383

Use stricter cache check in load.c

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

Status:
Closed
Priority:
Normal
Target version:
ruby -v:
ruby 2.0.0dev (2012-11-18 trunk 37708) [x86_64-linux]
[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

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

Associated revisions

Revision 97ecab7b
Added by shirosaki over 5 years ago

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.
    [Bug #7383]

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

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@37808 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 37808
Added by shirosaki over 5 years ago

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.
    [Bug #7383]

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

Revision 37808
Added by shirosaki over 5 years ago

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.
    [Bug #7383]

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

Revision 37808
Added by shirosaki over 5 years ago

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.
    [Bug #7383]

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

Revision 37808
Added by shirosaki over 5 years ago

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.
    [Bug #7383]

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

Revision 37808
Added by shirosaki over 5 years ago

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.
    [Bug #7383]

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

Revision e256dca8
Added by drbrain (Eric Hodel) over 5 years ago

  • Backport r39213 from trunk [ruby-trunk - Bug #7383]

  • lib/rubygems.rb: Return BINARY strings from Gem.gzip and Gem.gunzip.
    Fixes intermittent test failures. RubyGems issue #450 by Jeremey
    Kemper.

  • test/rubygems/test_gem.rb: Test for the above.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_0_0@39229 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

History

#1 [ruby-core:49581] Updated by h.shirosaki (Hiroshi Shirosaki) over 5 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?

#2 [ruby-core:49871] Updated by funny_falcon (Yura Sokolov) over 5 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) 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 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/

#3 [ruby-core:49881] Updated by h.shirosaki (Hiroshi Shirosaki) over 5 years ago

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

#4 Updated by Anonymous over 5 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.
    [Bug #7383]

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

Also available in: Atom PDF