Project

General

Profile

Actions

Bug #7304

closed

Random test failures around test_autoclose_true_closed_by_finalizer

Added by luislavena (Luis Lavena) over 11 years ago. Updated over 11 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 2.0.0dev (2012-11-07 trunk 37538) [i386-mingw32]
Backport:
[ruby-core:49044]

Description

=begin
Hello,

Over the past few days I've seen on and off failures on RubyInstaller CI related to (({test_autoclose_true_closed_by_finalizer})):

http://ci.rubyinstaller.org/job/ruby-trunk-x86-test-all/265/console

  1. Error:
    test_autoclose_true_closed_by_finalizer(TestIO):
    NoMethodError: undefined method close' for 2012-11-07 04:43:41 -0300:WeakRef C:/Users/Worker/Jenkins/workspace/ruby-trunk-x86-build/test/ruby/test_io.rb:1611:in test_autoclose_true_closed_by_finalizer'

This seems to happen when the system is under heavy load (because is running other jobs in parallel).

This might be a hint of something not working properly under heavy load, perhaps the GC in effect.

I was unable to produce the same failure on x64-mingw32, and haven't tried yet OSX or Linux.

Any ideas?
=end


Files

0001-Fix-WeakRef-finalize.patch (3.42 KB) 0001-Fix-WeakRef-finalize.patch h.shirosaki (Hiroshi Shirosaki), 11/09/2012 06:33 PM

Updated by luislavena (Luis Lavena) over 11 years ago

  • Status changed from Open to Assigned
  • ruby -v changed from ruby -v: ruby 2.0.0dev (2012-11-07 trunk 37538) [i386-mingw32] to ruby 2.0.0dev (2012-11-07 trunk 37538) [i386-mingw32]

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

I cannot reproduce above error. However, Bug #4168 and #5350 seem not solved.
I got NoMethodError by the following script. WeakRef object has reference to different object from originally associated.

% cat test_weakref.rb
require "weakref"

class Foo
  def foo; end
end

a = []
1000.times do
  a << WeakRef.new(Foo.new)
end

a.each do |x|
  begin
    x.foo
  rescue WeakRef::RefError
    p :referr
  end
end
% ruby -v test_weakref.rb
ruby 2.0.0dev (2012-11-08 trunk 37558) [i686-linux]
test_weakref.rb:14:in `block in <main>': undefined method `foo' for [70032780]:WeakRef (NoMethodError)
        from test_weakref.rb:12:in `each'
        from test_weakref.rb:12:in `<main>'

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

After some investigation, I found WeakRef finalize code appears wrong.
When finalize, object references were not removed from weakmap hash properly.

I attached a patch. I tested it with ruby 2.0.0dev (2012-11-09 trunk 37558) [i686-linux].

Updated by luislavena (Luis Lavena) over 11 years ago

  • Assignee changed from ko1 (Koichi Sasada) to authorNari (Narihiro Nakamura)
  • Priority changed from Normal to 5

Thank you Shirosaki-san,

Applying the patch, it fixes the WeakRef issues.

ruby -v: ruby 2.0.0dev (2012-11-10 trunk 37612) [i386-mingw32]
3 tests, 4 assertions, 0 failures, 0 errors, 0 skips

Reassigning to Narihiro Nakamura, as the changes seems to be GC-related?

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

  • Category changed from test to core
  • Assignee changed from authorNari (Narihiro Nakamura) to h.shirosaki (Hiroshi Shirosaki)

Would you split the patch into refactor by renaming and the fix?

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

  • Assignee changed from h.shirosaki (Hiroshi Shirosaki) to nobu (Nobuyoshi Nakada)

I've splited the patch into two commits and pushed it to github.

Could you check it? Thank you.
https://github.com/shirosaki/ruby/compare/trunk...weakref

Updated by luislavena (Luis Lavena) over 11 years ago

Hello Nobu,

As pointed by Shirosaki-san, the two commits are now split.

Can we apply those changes to trunk? Who should be assigned to final approval?

Thank you

Updated by luislavena (Luis Lavena) over 11 years ago

  • Assignee changed from nobu (Nobuyoshi Nakada) to h.shirosaki (Hiroshi Shirosaki)

Thank you Nobu,

Hiroshi, Nobu give you green light to commit the changes from the branch.

Thank you both!

Actions #10

Updated by Anonymous over 11 years ago

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

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


Fix finalize of WeakRef

  • gc.c (wmap_final_func): remove WeakRef object reference from the
    array.

  • gc.c (wmap_finalize): remove recycled object references from weak
    map hash properly. How to get object reference from object id was
    wrong. st_delete() doesn't work properly if key and value arguments
    are same. The key of obj2wmap is referenced object and the value of
    obj2wmap is WeakRef array.

  • gc.c (wmap_aset): obj2wmap should contain WeakRef array in the
    definition.

  • test/test_weakref.rb (TestWeakRef#test_not_reference_different_object): add a test for
    above.
    [ruby-core:49044] [Bug #7304]

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

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

This fix causes segv, which was pointed out at r37831. Thank you, naruse-san.
I found rb_ary_delete(ary, obj) is not usable when doing WeakRef finalize because rb_ary_delete() calls rb_equal() against GC'ed WeakRef object.
Instead just comparing VALUE by == seems good for this case. I'll fix later.

Actions #12

Updated by Anonymous over 11 years ago

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

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


Fix WeakRef finalize

  • array.c (rb_ary_delete_same_obj): new function for WeakRef.
    This deletes same objects as item argument in the array.

  • internal.h (rb_ary_delete_same_obj): add a declaration.

  • gc.c (wmap_final_func): remove WeakRef object reference from the
    array. rb_ary_delete() is not usable because it uses rb_equal() to
    compare object references.

  • gc.c (wmap_finalize): remove recycled object references from weak
    map hash properly. How to get object reference from object id was
    wrong. st_delete() doesn't work properly if key and value arguments
    are same. The key of obj2wmap is referenced object and the value of
    obj2wmap is WeakRef array.

  • gc.c (wmap_aset): obj2wmap should contain WeakRef array in the
    definition.

  • test/test_weakref.rb (TestWeakRef#test_not_reference_different_object,
    TestWeakRef#test_weakref_finalize): add tests for above.
    [ruby-core:49044] [Bug #7304]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0