From: Nobuyoshi Nakada <nobu@...>
Date: 2011-09-29T20:48:38+09:00
Subject: [ruby-dev:44562] Re: [ruby-dev:44538] [Ruby 1.9 - Bug #5350] WeakRef で謎の NoMethodError

なかだです。

At Sat, 24 Sep 2011 11:38:54 +0900,
SASADA Koichi wrote in [ruby-dev:44538]:
> (2011/09/23 19:02), SASADA Koichi wrote:
>> 　うーん，これはどうするべきかな．ファイナライズ処理を遅延させるように
>> コードを書き換えれば解決できますが，ちょっと大がかりな気もしますね．問題
>> が weakref だけなら，大がかりでもいい気がしますが．
> 
> 　わかりづらい文章になってしまってすみません．weakref 側を，ファイナライ
> ザ処理を遅延できるように大がかりに書き換えればよい，という意図でした．例
> えば，ファイナライザはこの処理をファイナライザの *後で* 実行するように，
> 例えば Thread 作っちゃうとかすれば解決できます．

weakrefのマップからの削除処理は、ファイナライザから遅延させてはまずいで
す。削除が完了する前にファイナライザが終了してしまうと、対象のオブジェ
クトは再利用される可能性があり、その時点でweakrefから参照すると意図しな
いオブジェクトが得られることになります。

第一の問題点は、weakrefのファイナライズ処理が再入不能なのに、ファイナラ
イザ自体は再入することがあり得ることです。[ruby-dev:44527]にある再現コー
ドのように、weakrefのファイナライズ処理で内部的に使っているMutexが外部
からも容易にアクセスできてしまうことも問題といっていいでしょう。

とりあえずファイナライザで再入しないようにするパッチです。


diff --git i/gc.c w/gc.c
index f9a945c..fad49e0 100644
--- i/gc.c
+++ w/gc.c
@@ -345,6 +345,7 @@ typedef struct rb_objspace {
 	int dont_gc;
 	int dont_lazy_sweep;
 	int during_gc;
+	rb_atomic_t finalizing;
     } flags;
     struct {
 	st_table *table;
@@ -387,6 +388,7 @@ int *ruby_initial_gc_stress_ptr = &rb_objspace.gc_stress;
 #define heaps_freed		objspace->heap.freed
 #define dont_gc 		objspace->flags.dont_gc
 #define during_gc		objspace->flags.during_gc
+#define finalizing		objspace->flags.finalizing
 #define finalizer_table 	objspace->final.table
 #define deferred_final_list	objspace->final.deferred
 #define mark_stack		objspace->markstack.buffer
@@ -2064,7 +2066,7 @@ slot_sweep(rb_objspace_t *objspace, struct heaps_slot *sweep_slot)
     }
     objspace->heap.final_num += final_num;
 
-    if (deferred_final_list) {
+    if (deferred_final_list && !finalizing) {
         rb_thread_t *th = GET_THREAD();
         if (th) {
             RUBY_VM_SET_FINALIZER_INTERRUPT(th);
@@ -2968,7 +2970,10 @@ finalize_deferred(rb_objspace_t *objspace)
 void
 rb_gc_finalize_deferred(void)
 {
-    finalize_deferred(&rb_objspace);
+    rb_objspace_t *objspace = &rb_objspace;
+    if (ATOMIC_SET(finalizing, 1)) return;
+    finalize_deferred(objspace);
+    ATOMIC_SET(finalizing, 0);
 }
 
 static int
@@ -3020,6 +3025,8 @@ rb_objspace_call_finalizer(rb_objspace_t *objspace)
     /* run finalizers */
     gc_clear_mark_on_sweep_slots(objspace);
 
+    if (ATOMIC_SET(finalizing, 1)) return;
+
     do {
 	/* XXX: this loop will make no sense */
 	/* because mark will not be removed */
@@ -3082,6 +3089,7 @@ rb_objspace_call_finalizer(rb_objspace_t *objspace)
 
     st_free_table(finalizer_table);
     finalizer_table = 0;
+    ATOMIC_SET(finalizing, 0);
 }
 
 void
@@ -3089,7 +3097,7 @@ rb_gc(void)
 {
     rb_objspace_t *objspace = &rb_objspace;
     garbage_collect(objspace);
-    finalize_deferred(objspace);
+    if (!finalizing) finalize_deferred(objspace);
     free_unused_heaps(objspace);
 }
 


-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦