ruby-changes:25777
From: shirosaki <ko1@a...>
Date: Sat, 24 Nov 2012 21:27:06 +0900 (JST)
Subject: [ruby-changes:25777] shirosaki:r37834 (trunk): Fix WeakRef finalize
shirosaki 2012-11-24 21:26:54 +0900 (Sat, 24 Nov 2012) New Revision: 37834 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=37834 Log: 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] Modified files: trunk/ChangeLog trunk/array.c trunk/gc.c trunk/internal.h trunk/test/test_weakref.rb Index: array.c =================================================================== --- array.c (revision 37833) +++ array.c (revision 37834) @@ -2777,6 +2777,40 @@ } VALUE +rb_ary_delete_same_obj(VALUE ary, VALUE item) +{ + VALUE v = item; + long i1, i2; + + for (i1 = i2 = 0; i1 < RARRAY_LEN(ary); i1++) { + VALUE e = RARRAY_PTR(ary)[i1]; + + if (e == item) { + v = e; + continue; + } + if (i1 != i2) { + rb_ary_store(ary, i2, e); + } + i2++; + } + if (RARRAY_LEN(ary) == i2) { + return Qnil; + } + + rb_ary_modify(ary); + if (RARRAY_LEN(ary) > i2) { + ARY_SET_LEN(ary, i2); + if (i2 * 2 < ARY_CAPA(ary) && + ARY_CAPA(ary) > ARY_DEFAULT_SIZE) { + ary_resize_capa(ary, i2*2); + } + } + + return v; +} + +VALUE rb_ary_delete_at(VALUE ary, long pos) { long len = RARRAY_LEN(ary); Index: ChangeLog =================================================================== --- ChangeLog (revision 37833) +++ ChangeLog (revision 37834) @@ -1,3 +1,28 @@ +Sat Nov 24 21:01:55 2012 Hiroshi Shirosaki <h.shirosaki@g...> + + * 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] + Sat Nov 24 19:44:41 2012 NARUSE, Yui <naruse@r...> * ext/nkf/nkf-utf8/nkf.c (unicode_iconv_combine): returning flags are Index: gc.c =================================================================== --- gc.c (revision 37833) +++ gc.c (revision 37834) @@ -3751,8 +3751,8 @@ { VALUE obj, ary; if (!existing) return ST_STOP; - obj = (VALUE)*key, ary = (VALUE)*value; - rb_ary_delete(ary, obj); + obj = (VALUE)arg, ary = (VALUE)*value; + rb_ary_delete_same_obj(ary, obj); if (!RARRAY_LEN(ary)) return ST_DELETE; return ST_CONTINUE; } @@ -3760,16 +3760,18 @@ static VALUE wmap_finalize(VALUE self, VALUE obj) { - st_data_t data; + st_data_t key, data; VALUE rids; long i; struct weakmap *w; TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); - obj = NUM2PTR(obj); + /* Get reference from object id. */ + obj = obj ^ FIXNUM_FLAG; /* unset FIXNUM_FLAG */ - data = (st_data_t)obj; - if (st_delete(w->obj2wmap, &data, &data)) { + /* obj is original referenced object and/or weak reference. */ + key = (st_data_t)obj; + if (st_delete(w->obj2wmap, &key, &data)) { rids = (VALUE)data; for (i = 0; i < RARRAY_LEN(rids); ++i) { data = (st_data_t)RARRAY_PTR(rids)[i]; @@ -3777,9 +3779,9 @@ } } - data = (st_data_t)obj; - if (st_delete(w->wmap2obj, &data, &data)) { - st_update(w->obj2wmap, (st_data_t)obj, wmap_final_func, 0); + key = (st_data_t)obj; + if (st_delete(w->wmap2obj, &key, &data)) { + st_update(w->obj2wmap, data, wmap_final_func, (st_data_t)obj); } return self; } @@ -3801,7 +3803,7 @@ rids = rb_ary_tmp_new(1); st_insert(w->obj2wmap, (st_data_t)orig, (st_data_t)rids); } - rb_ary_push(rids, orig); + rb_ary_push(rids, wmap); st_insert(w->wmap2obj, (st_data_t)wmap, (st_data_t)orig); return nonspecial_obj_id(orig); } Index: internal.h =================================================================== --- internal.h (revision 37833) +++ internal.h (revision 37834) @@ -48,6 +48,7 @@ VALUE rb_ary_last(int, VALUE *, VALUE); void rb_ary_set_len(VALUE, long); VALUE rb_ary_cat(VALUE, const VALUE *, long); +VALUE rb_ary_delete_same_obj(VALUE, VALUE); /* bignum.c */ VALUE rb_big_fdiv(VALUE x, VALUE y); Index: test/test_weakref.rb =================================================================== --- test/test_weakref.rb (revision 37833) +++ test/test_weakref.rb (revision 37834) @@ -1,5 +1,6 @@ require 'test/unit' require 'weakref' +require_relative './ruby/envutil' class TestWeakRef < Test::Unit::TestCase def make_weakref(level = 10) @@ -21,4 +22,35 @@ ObjectSpace.garbage_collect assert_raise(WeakRef::RefError) {weak.to_s} end + + def test_not_reference_different_object + bug7304 = '[ruby-core:49044]' + weakrefs = [] + 3.times do + obj = Object.new + def obj.foo; end + weakrefs << WeakRef.new(obj) + ObjectSpace.garbage_collect + end + assert_nothing_raised(NoMethodError, bug7304) { + weakrefs.each do |weak| + begin + weak.foo + rescue WeakRef::RefError + end + end + } + end + + def test_weakref_finalize + bug7304 = '[ruby-core:49044]' + assert_normal_exit %q{ + require 'weakref' + obj = Object.new + 3.times do + WeakRef.new(obj) + ObjectSpace.garbage_collect + end + }, bug7304 + end end -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/