shirosaki	2012-11-24 13:01:33 +0900 (Sat, 24 Nov 2012)

  New Revision: 37826

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=37826

  Log:
    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]

  Modified files:
    trunk/ChangeLog
    trunk/gc.c
    trunk/test/test_weakref.rb

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 37825)
+++ ChangeLog	(revision 37826)
@@ -1,3 +1,22 @@
+Sat Nov 24 12:10:26 2012  Hiroshi Shirosaki  <h.shirosaki@g...>
+
+	* 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]
+
 Sat Nov 24 11:47:14 2012  KOSAKI Motohiro  <kosaki.motohiro@g...>
 
 	* process.c (proc_getsid): adds new method for getting session id.
Index: gc.c
===================================================================
--- gc.c	(revision 37825)
+++ gc.c	(revision 37826)
@@ -3751,7 +3751,7 @@
 {
     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);
     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: test/test_weakref.rb
===================================================================
--- test/test_weakref.rb	(revision 37825)
+++ test/test_weakref.rb	(revision 37826)
@@ -21,4 +21,23 @@
     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
 end

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/