Permalink
Browse files

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]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@37834 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
  • Loading branch information...
1 parent e6c2ffa commit 1cdeab5cdd4b84cc0d97ee1a148beeb11bdd2d44 @shirosaki shirosaki committed Nov 24, 2012
Showing with 104 additions and 10 deletions.
  1. +25 −0 ChangeLog
  2. +34 −0 array.c
  3. +12 −10 gc.c
  4. +1 −0 internal.h
  5. +32 −0 test/test_weakref.rb
View
@@ -1,3 +1,28 @@
+Sat Nov 24 21:01:55 2012 Hiroshi Shirosaki <h.shirosaki@gmail.com>
+
+ * 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@ruby-lang.org>
* ext/nkf/nkf-utf8/nkf.c (unicode_iconv_combine): returning flags are
View
@@ -2777,6 +2777,40 @@ rb_ary_delete(VALUE ary, VALUE item)
}
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);
View
@@ -3751,35 +3751,37 @@ wmap_final_func(st_data_t *key, st_data_t *value, st_data_t arg, int existing)
{
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;
}
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];
st_delete(w->wmap2obj, &data, NULL);
}
}
- 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 @@ wmap_aset(VALUE self, VALUE wmap, VALUE orig)
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);
}
View
@@ -48,6 +48,7 @@ struct vtm; /* defined by timev.h */
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);
View
@@ -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 @@ def test_recycled
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

0 comments on commit 1cdeab5

Please sign in to comment.