Bug #369: Ruby 1.9でRailsのactionpackのテストの終了時にSEGV

すみません！タイトルがひどいことになってしまいました・・・

どなたか権限のある方、タイトルを「Ruby 1.9でRailsのactionpackのテストの終了時にSEGV」とか
そのような形に書き換えていただけると幸いです。

起票時点のtrunkの
ruby 1.9.0 (2008-07-29 revision 18256) [i686-darwin9.4.0]
でも再現しました。

ワナベと申します。

2008/07/29 18:28 Akira Matsuda <[e-mail]>:
> Ruby1.9にてRuby on Railsの中のactionpackのテストを走らせると、終了後にcoreを吐きながらアボートします。
> Rubyのバージョンは1.9.0-3で、Railsはgitリポジトリ上の報告時現在のedgeバージョンにて再現可能です。
> また、OSはOSX LeopardおよびDebian etchにて再現済みです。

以下で発生するSEGVと同様なのではないかと思います。
ファイナライザが複数定義されているとき、すでにdfreeが呼ばれた
オブジェクトを参照すると起きる問題のようです。

$ ./ruby -ve '
a1,a2,b1,b2=Array.new(4){""}
ObjectSpace.define_finalizer(b2,proc{p :b2})
ObjectSpace.define_finalizer(b1,proc{p :b1, b1})

ObjectSpace.define_finalizer(a2,proc{p :a2, a1})
ObjectSpace.define_finalizer(a1,proc{p :a1})
'
ruby 1.9.0 (2008-08-06 revision 18383) [i386-mingw32]
:a1
:a2
[2014068]
:b1
-e:4: [BUG] Segmentation fault
ruby 1.9.0 (2008-08-06 revision 18383) [i386-mingw32]

-- control frame ----------
c:0005 p:---- s:0011 b:0011 l:000010 d:000010 CFUNC  :p
c:0004 p:0012 s:0006 b:0006 l:000e5c d:000005 BLOCK  -e:4
c:0003 p:---- s:0006 b:0006 l:000005 d:000005 FINISH :set_backtrace
c:0002 p:---- s:0004 b:0004 l:000003 d:000003 CFUNC  :call
c:0001 p:0000 s:0002 b:0002 l:000001 d:000001 TOP    <dummy toplevel>:483
---------------------------
DBG> : "-e:4:in `p'"
DBG> : "-e:4:in `block in <main>'"
DBG> : ":0:in `call'"

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.

-- 
ワナベ

なかだです。

At Wed, 6 Aug 2008 16:06:04 +0900,
wanabe wrote in [ruby-dev:35778]:
> 2008/07/29 18:28 Akira Matsuda <[e-mail]>:
> > Ruby1.9にてRuby on Railsの中のactionpackのテストを走らせると、終了後にcoreを吐きながらアボートします。
> > Rubyのバージョンは1.9.0-3で、Railsはgitリポジトリ上の報告時現在のedgeバージョンにて再現可能です。
> > また、OSはOSX LeopardおよびDebian etchにて再現済みです。
> 
> 以下で発生するSEGVと同様なのではないかと思います。
> ファイナライザが複数定義されているとき、すでにdfreeが呼ばれた
> オブジェクトを参照すると起きる問題のようです。
> 
> $ ./ruby -ve '
> a1,a2,b1,b2=Array.new(4){""}
> ObjectSpace.define_finalizer(b2,proc{p :b2})
> ObjectSpace.define_finalizer(b1,proc{p :b1, b1})
> 
> ObjectSpace.define_finalizer(a2,proc{p :a2, a1})
> ObjectSpace.define_finalizer(a1,proc{p :a1})
> '
> ruby 1.9.0 (2008-08-06 revision 18383) [i386-mingw32]
> :a1
> :a2
> [2014068]
> :b1
> -e:4: [BUG] Segmentation fault

本当にRailsにそういったコードがあるとすると、いったい何をしたい
んでしょうか。これらのファイナライザは、対象オブジェクト自身を参
照しているので、どれも呼ばれないというのが正しい動作だと思います。


Index: gc.c
===================================================================
--- gc.c	(revision 18386)
+++ gc.c	(working copy)
@@ -2092,5 +2092,5 @@ chain_finalized_object(st_data_t key, st
 {
     RVALUE *p = (RVALUE *)key, **final_list = (RVALUE **)arg;
-    if (p->as.basic.flags & FL_FINALIZE) {
+    if ((p->as.basic.flags & (FL_FINALIZE|FL_MARK)) == FL_FINALIZE) {
 	if (BUILTIN_TYPE(p) != T_DEFERRED) {
 	    p->as.free.flags = FL_MARK | T_DEFERRED; /* remain marked */
@@ -2116,4 +2116,5 @@ rb_gc_call_finalizer_at_exit(void)
 	    deferred_final_list = 0;
 	    finalize_list(objspace, p);
+	    mark_tbl(objspace, finalizer_table, 0);
 	    st_foreach(finalizer_table, chain_finalized_object,
 		       (st_data_t)&deferred_final_list);


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

なかだです。

At Wed, 6 Aug 2008 17:17:30 +0900,
Nobuyoshi Nakada wrote in [ruby-dev:35780]:
> 本当にRailsにそういったコードがあるとすると、いったい何をしたい
> んでしょうか。これらのファイナライザは、対象オブジェクト自身を参
> 照しているので、どれも呼ばれないというのが正しい動作だと思います。

1.8用のパッチです。


Index: common.mk
===================================================================
--- common.mk	(revision 18383)
+++ common.mk	(working copy)
@@ -391,5 +391,5 @@ file.$(OBJEXT): {$(VPATH)}file.c $(RUBY_
   {$(VPATH)}rubyio.h {$(VPATH)}rubysig.h {$(VPATH)}util.h \
   {$(VPATH)}dln.h
-gc.$(OBJEXT): {$(VPATH)}gc.c $(RUBY_H_INCLUDES) \
+gc.$(OBJEXT): {$(VPATH)}gc.c $(RUBY_H_INCLUDES) {$(VPATH)}rubyio.h \
   {$(VPATH)}rubysig.h {$(VPATH)}st.h {$(VPATH)}node.h \
   {$(VPATH)}env.h {$(VPATH)}re.h {$(VPATH)}regex.h
Index: gc.c
===================================================================
--- gc.c	(revision 18383)
+++ gc.c	(working copy)
@@ -14,4 +14,5 @@

 #include "ruby.h"
+#include "rubyio.h"
 #include "rubysig.h"
 #include "st.h"
@@ -1063,5 +1064,14 @@ gc_mark_children(ptr, lev)
 }

-static void obj_free _((VALUE));
+static int obj_free _((VALUE));
+
+static inline void
+add_freelist(p)
+    RVALUE *p;
+{
+    p->as.free.flags = 0;
+    p->as.free.next = freelist;
+    freelist = p;
+}

 static void
@@ -1073,7 +1083,5 @@ finalize_list(p)
 	run_final((VALUE)p);
 	if (!FL_TEST(p, FL_SINGLETON)) { /* not freeing page */
-	    p->as.free.flags = 0;
-	    p->as.free.next = freelist;
-	    freelist = p;
+	    add_freelist(p);
 	}
 	p = tmp;
@@ -1100,4 +1108,6 @@ free_unused_heaps()
 }

+#define T_DEFERRED 0x3a
+
 void rb_gc_abort_threads(void);

@@ -1143,24 +1153,26 @@ gc_sweep()
 	RVALUE *free = freelist;
 	RVALUE *final = final_list;
+	int deferred;

 	p = heaps[i].slot; pend = p + heaps[i].limit;
 	while (p < pend) {
 	    if (!(p->as.basic.flags & FL_MARK)) {
-		if (p->as.basic.flags) {
-		    obj_free((VALUE)p);
-		}
-		if (need_call_final && FL_TEST(p, FL_FINALIZE)) {
-		    p->as.free.flags = FL_MARK; /* remain marked */
+		if (p->as.basic.flags &&
+		    ((deferred = obj_free((VALUE)p)) ||
+		     ((FL_TEST(p, FL_FINALIZE)) && need_call_final))) {
+		    if (!deferred) {
+			p->as.free.flags = T_DEFERRED;
+			RDATA(p)->dfree = 0;
+		    }
+		    p->as.free.flags |= FL_MARK;
 		    p->as.free.next = final_list;
 		    final_list = p;
 		}
 		else {
-		    p->as.free.flags = 0;
-		    p->as.free.next = freelist;
-		    freelist = p;
+		    add_freelist(p);
 		}
 		n++;
 	    }
-	    else if (RBASIC(p)->flags == FL_MARK) {
+	    else if (BUILTIN_TYPE(p) == T_DEFERRED) {
 		/* objects to be finalized */
 		/* do nothing remain marked */
@@ -1208,14 +1220,19 @@ rb_gc_force_recycle(p)
     VALUE p;
 {
-    RANY(p)->as.free.flags = 0;
-    RANY(p)->as.free.next = freelist;
-    freelist = RANY(p);
+    add_freelist(p);
 }

-static void
+static inline void
+make_deferred(p)
+    RVALUE *p;
+{
+    p->as.basic.flags = (p->as.basic.flags & ~T_MASK) | T_DEFERRED;
+}
+
+static int
 obj_free(obj)
     VALUE obj;
 {
-    switch (RANY(obj)->as.basic.flags & T_MASK) {
+    switch (BUILTIN_TYPE(obj)) {
       case T_NIL:
       case T_FIXNUM:
@@ -1230,5 +1247,5 @@ obj_free(obj)
     }

-    switch (RANY(obj)->as.basic.flags & T_MASK) {
+    switch (BUILTIN_TYPE(obj)) {
       case T_OBJECT:
 	if (RANY(obj)->as.object.iv_tbl) {
@@ -1273,5 +1290,6 @@ obj_free(obj)
 	    }
 	    else if (RANY(obj)->as.data.dfree) {
-		(*RANY(obj)->as.data.dfree)(DATA_PTR(obj));
+		make_deferred(RANY(obj));
+		return 1;
 	    }
 	}
@@ -1285,6 +1303,9 @@ obj_free(obj)
       case T_FILE:
 	if (RANY(obj)->as.file.fptr) {
-	    rb_io_fptr_finalize(RANY(obj)->as.file.fptr);
-	    RUBY_CRITICAL(free(RANY(obj)->as.file.fptr));
+	    rb_io_t *fptr = RANY(obj)->as.file.fptr;
+	    make_deferred(RANY(obj));
+	    RDATA(obj)->dfree = (void (*)(void*))rb_io_fptr_finalize;
+	    RDATA(obj)->data = fptr;
+	    return 1;
 	}
 	break;
@@ -1314,5 +1335,5 @@ obj_free(obj)
 	    break;
 	}
-	return;			/* no need to free iv_tbl */
+	break;			/* no need to free iv_tbl */

       case T_SCOPE:
@@ -1337,4 +1358,6 @@ obj_free(obj)
 	       RANY(obj)->as.basic.flags & T_MASK, obj);
     }
+
+    return 0;
 }

@@ -1682,4 +1705,5 @@ os_obj_of(of)
 		  case T_SCOPE:
 		  case T_NODE:
+		  case T_DEFERRED:
 		    continue;
 		  case T_CLASS:
@@ -1929,4 +1953,19 @@ rb_gc_finalize_deferred()
 }

+static int
+chain_finalized_object(st_data_t key, st_data_t val, st_data_t arg)
+{
+    RVALUE *p = (RVALUE *)key, **final_list = (RVALUE **)arg;
+    if ((p->as.basic.flags & (FL_FINALIZE|FL_MARK)) == FL_FINALIZE) {
+	if (BUILTIN_TYPE(p) != T_DEFERRED) {
+	    p->as.free.flags = FL_MARK | T_DEFERRED; /* remain marked */
+	    RDATA(p)->dfree = 0;
+	}
+	p->as.free.next = *final_list;
+	*final_list = p;
+    }
+    return ST_CONTINUE;
+}
+
 void
 rb_gc_call_finalizer_at_exit()
@@ -1937,18 +1976,12 @@ rb_gc_call_finalizer_at_exit()
     /* run finalizers */
     if (need_call_final) {
-	p = deferred_final_list;
-	deferred_final_list = 0;
-	finalize_list(p);
-	for (i = 0; i < heaps_used; i++) {
-	    p = heaps[i].slot; pend = p + heaps[i].limit;
-	    while (p < pend) {
-		if (FL_TEST(p, FL_FINALIZE)) {
-		    FL_UNSET(p, FL_FINALIZE);
-		    p->as.basic.klass = 0;
-		    run_final((VALUE)p);
-		}
-		p++;
-	    }
-	}
+	do {
+	    p = deferred_final_list;
+	    deferred_final_list = 0;
+	    finalize_list(p);
+	    mark_tbl(finalizer_table, 0);
+	    st_foreach(finalizer_table, chain_finalized_object,
+		       (st_data_t)&deferred_final_list);
+	} while (deferred_final_list);
     }
     /* run data object's finalizers */


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

報告者の松田です。

ご対応ありがとうございます。
なかださんのRuby 1.9のほうのパッチを当てて試してみたところ、
確かに落ちないようになりました。

# 1.8のほうに関しては、もともと同様の現象が出ていなかったので
# こちらでは特に検証はできていません。

 > 本当にRailsにそういったコードがあるとすると、いった 
い何をしたいんでしょうか。

Railsの側にも問題がありそう、と、なかださんに言われたので、
もう少し追ってみました。

この件、最小ケースとしては、Railsのソースディレクトリで
$ cd actionpack
$ ruby19 -Ilib:test "test/controller/session/cookie_store_test.rb"
にて再現可能で、この中の、test_restores_double_encoded_cookies
というテストケースで落ちていることがわかりました。

また、actionpack/lib/action_controller/session/cookie_store.rb
の159行目あたりの、
  @session.cgi.send :instance_variable_set, '@output_cookies', [cookie]
というコードをコメントアウトすればどうやら落ちなくなる、
ということまではわかりました。

せっかくなので、どなたか以上の情報で何かお気づきの点があれば
フィードバックをいただければ幸いです。

ただし、この場はあくまでruby-devであって、
このRedmineはRailsのバグを直すためのITSでは 
ないので、
Ruby的にはこのチケットはこれにて解決、
ということにしていただいて、もちろん構いません。

--
Akira Matsuda<[e-mail]>

• Status changed from Open to Closed

Applied in changeset r18398