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

ChangeLog

@@ -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

array.c

@@ -2776,6 +2776,40 @@ rb_ary_delete(VALUE ary, VALUE item)
     return v;
 }
 
+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)
 {

gc.c

@@ -3751,8 +3751,8 @@ 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;
+    obj = (VALUE)arg, ary = (VALUE)*value;
-    rb_ary_delete(ary, obj);
+    rb_ary_delete_same_obj(ary, obj);
     if (!RARRAY_LEN(ary)) return ST_DELETE;
     return ST_CONTINUE;
 }
@@ -3760,16 +3760,18 @@ wmap_final_func(st_data_t *key, st_data_t *value, st_data_t arg, int existing)
 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;
+    /* obj is original referenced object and/or weak reference. */
-    if (st_delete(w->obj2wmap, &data, &data)) {
+    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 @@ wmap_finalize(VALUE self, VALUE obj)
 	}
     }
 
-    data = (st_data_t)obj;
+    key = (st_data_t)obj;
-    if (st_delete(w->wmap2obj, &data, &data)) {
+    if (st_delete(w->wmap2obj, &key, &data)) {
-	st_update(w->obj2wmap, (st_data_t)obj, wmap_final_func, 0);
+	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);
 }

internal.h

@@ -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);

test/test_weakref.rb

@@ -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 @@ class TestWeakRef < Test::Unit::TestCase
     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