[前][次][番号順一覧][スレッド一覧]

ruby-changes:37850

From: ko1 <ko1@a...>
Date: Wed, 11 Mar 2015 18:15:32 +0900 (JST)
Subject: [ruby-changes:37850] ko1:r49931 (trunk): * gc.c: fix memory leak by prepend method.

ko1	2015-03-11 18:15:20 +0900 (Wed, 11 Mar 2015)

  New Revision: 49931

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

  Log:
    * gc.c: fix memory leak by prepend method.
      It is easy to reproduce with such script:
        module M; def bar; end; end
        loop{
          Class.new do
            def foo; end
            prepend M
          end
        }
    * gc.c (obj_free): free T_ICLASS::m_tbl if it is created by prepend.
      To recognize it, check RICLASS_IS_ORIGIN flag.
    * gc.c (gc_mark_children): T_ICLASS objects only need to mark
      T_ICLASS::m_tbl if RICLASS_IS_ORIGIN is set.
    * gc.c (obj_memsize_of): count T_ICLASS if RICLASS_IS_ORIGIN is set.
    * internal.h (RCLASS_SET_ORIGIN): add to set RCLASS_SET_ORIGIN.
      TODO: The word `origin' seems not good name. We need to invent
      another good name.
    * class.c: use RCLASS_SET_ORIGIN().
    * class.c (class_alloc): zero clear rb_classext_t.

  Modified files:
    trunk/ChangeLog
    trunk/class.c
    trunk/gc.c
    trunk/internal.h
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 49930)
+++ ChangeLog	(revision 49931)
@@ -1,3 +1,34 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Wed Mar 11 17:03:20 2015  Koichi Sasada  <ko1@a...>
+
+	* gc.c: fix memory leak by prepend method.
+
+	  It is easy to reproduce with such script:
+
+	    module M; def bar; end; end
+	    loop{
+	      Class.new do
+	        def foo; end
+	        prepend M
+	      end
+	    }
+
+	* gc.c (obj_free): free T_ICLASS::m_tbl if it is created by prepend.
+	  To recognize it, check RICLASS_IS_ORIGIN flag.
+
+	* gc.c (gc_mark_children): T_ICLASS objects only need to mark
+	  T_ICLASS::m_tbl if RICLASS_IS_ORIGIN is set.
+
+	* gc.c (obj_memsize_of): count T_ICLASS if RICLASS_IS_ORIGIN is set.
+
+	* internal.h (RCLASS_SET_ORIGIN): add to set RCLASS_SET_ORIGIN.
+
+	  TODO: The word `origin' seems not good name. We need to invent
+	  another good name.
+
+	* class.c: use RCLASS_SET_ORIGIN().
+
+	* class.c (class_alloc): zero clear rb_classext_t.
+
 Wed Mar 11 13:28:49 2015  Nobuyoshi Nakada  <nobu@r...>
 
 	* configure.in: check also procstat_getvmmap, which is not
Index: gc.c
===================================================================
--- gc.c	(revision 49930)
+++ gc.c	(revision 49931)
@@ -1882,7 +1882,6 @@ obj_free(rb_objspace_t *objspace, VALUE https://github.com/ruby/ruby/blob/trunk/gc.c#L1882
       case T_MODULE:
       case T_CLASS:
 	rb_free_m_tbl(RCLASS_M_TBL(obj));
-
 	if (RCLASS_IV_TBL(obj)) {
 	    st_free_table(RCLASS_IV_TBL(obj));
 	}
@@ -1974,7 +1973,10 @@ obj_free(rb_objspace_t *objspace, VALUE https://github.com/ruby/ruby/blob/trunk/gc.c#L1973
       case T_COMPLEX:
 	break;
       case T_ICLASS:
-	/* iClass shares table with the module */
+	/* Basically , T_ICLASS shares table with the module */
+	if (FL_TEST(obj, RICLASS_IS_ORIGIN)) {
+	    rb_free_m_tbl(RCLASS_M_TBL(obj));
+	}
 	if (RCLASS_EXT(obj)->subclasses) {
 	    rb_class_detach_subclasses(obj);
 	    RCLASS_EXT(obj)->subclasses = NULL;
@@ -2874,6 +2876,13 @@ obj_memsize_of(VALUE obj, int use_all_ty https://github.com/ruby/ruby/blob/trunk/gc.c#L2876
 	    size += sizeof(rb_classext_t);
 	}
 	break;
+      case T_ICLASS:
+	if (FL_TEST(obj, RICLASS_IS_ORIGIN)) {
+	    if (RCLASS_M_TBL(obj)) {
+		size += st_memsize(RCLASS_M_TBL(obj));
+	    }
+	}
+	break;
       case T_STRING:
 	size += rb_str_memsize(obj);
 	break;
@@ -2909,9 +2918,6 @@ obj_memsize_of(VALUE obj, int use_all_ty https://github.com/ruby/ruby/blob/trunk/gc.c#L2918
       case T_RATIONAL:
       case T_COMPLEX:
 	break;
-      case T_ICLASS:
-	/* iClass shares table with the module */
-	break;
 
       case T_FLOAT:
       case T_SYMBOL:
@@ -4135,15 +4141,21 @@ gc_mark_children(rb_objspace_t *objspace https://github.com/ruby/ruby/blob/trunk/gc.c#L4141
     switch (BUILTIN_TYPE(obj)) {
       case T_CLASS:
       case T_MODULE:
-	if (!RCLASS_EXT(obj)) break;
-	mark_m_tbl(objspace, RCLASS_M_TBL(RCLASS_ORIGIN(obj)));
-      case T_ICLASS:
+	mark_m_tbl(objspace, RCLASS_M_TBL(obj));
 	if (!RCLASS_EXT(obj)) break;
 	mark_tbl(objspace, RCLASS_IV_TBL(obj));
 	mark_const_tbl(objspace, RCLASS_CONST_TBL(obj));
 	gc_mark(objspace, RCLASS_SUPER((VALUE)obj));
 	break;
 
+      case T_ICLASS:
+	if (FL_TEST(obj, RICLASS_IS_ORIGIN)) {
+	    mark_m_tbl(objspace, RCLASS_M_TBL(obj));
+	}
+	if (!RCLASS_EXT(obj)) break;
+	gc_mark(objspace, RCLASS_SUPER((VALUE)obj));
+	break;
+
       case T_ARRAY:
 	if (FL_TEST(obj, ELTS_SHARED)) {
 	    gc_mark(objspace, any->as.array.as.heap.aux.shared);
Index: class.c
===================================================================
--- class.c	(revision 49930)
+++ class.c	(revision 49931)
@@ -163,21 +163,22 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/class.c#L163
 class_alloc(VALUE flags, VALUE klass)
 {
     NEWOBJ_OF(obj, struct RClass, klass, (flags & T_MASK) | FL_PROMOTED1 /* start from age == 2 */ | (RGENGC_WB_PROTECTED_CLASS ? FL_WB_PROTECTED : 0));
-    obj->ptr = ALLOC(rb_classext_t);
-    RCLASS_IV_TBL(obj) = 0;
-    RCLASS_CONST_TBL(obj) = 0;
-    RCLASS_M_TBL(obj) = 0;
-    RCLASS_SET_SUPER((VALUE)obj, 0);
-    RCLASS_ORIGIN(obj) = (VALUE)obj;
-    RCLASS_IV_INDEX_TBL(obj) = 0;
-
-    RCLASS_EXT(obj)->subclasses = NULL;
-    RCLASS_EXT(obj)->parent_subclasses = NULL;
-    RCLASS_EXT(obj)->module_subclasses = NULL;
+    obj->ptr = ZALLOC(rb_classext_t);
+    /* ZALLOC
+      RCLASS_IV_TBL(obj) = 0;
+      RCLASS_CONST_TBL(obj) = 0;
+      RCLASS_M_TBL(obj) = 0;
+      RCLASS_IV_INDEX_TBL(obj) = 0;
+      RCLASS_SET_SUPER((VALUE)obj, 0);
+      RCLASS_EXT(obj)->subclasses = NULL;
+      RCLASS_EXT(obj)->parent_subclasses = NULL;
+      RCLASS_EXT(obj)->module_subclasses = NULL;
+     */
+    RCLASS_SET_ORIGIN((VALUE)obj, (VALUE)obj);
     RCLASS_SERIAL(obj) = rb_next_class_serial();
-
     RCLASS_REFINED_CLASS(obj) = Qnil;
     RCLASS_EXT(obj)->allocator = 0;
+
     return (VALUE)obj;
 }
 
@@ -939,7 +940,7 @@ rb_prepend_module(VALUE klass, VALUE mod https://github.com/ruby/ruby/blob/trunk/class.c#L940
 	OBJ_WB_UNPROTECT(origin); /* TODO: conservative shading. Need more survey. */
 	RCLASS_SET_SUPER(origin, RCLASS_SUPER(klass));
 	RCLASS_SET_SUPER(klass, origin);
-	RB_OBJ_WRITE(klass, &RCLASS_ORIGIN(klass), origin);
+	RCLASS_SET_ORIGIN(klass, origin);
 	RCLASS_M_TBL(origin) = RCLASS_M_TBL(klass);
 	RCLASS_M_TBL_INIT(klass);
 	st_foreach(RCLASS_M_TBL(origin), move_refined_method,
Index: internal.h
===================================================================
--- internal.h	(revision 49930)
+++ internal.h	(revision 49931)
@@ -311,7 +311,7 @@ struct rb_classext_struct { https://github.com/ruby/ruby/blob/trunk/internal.h#L311
      */
     rb_subclass_entry_t **module_subclasses;
     rb_serial_t class_serial;
-    VALUE origin;
+    const VALUE origin_;
     VALUE refined_class;
     rb_alloc_func_t allocator;
 };
@@ -477,10 +477,19 @@ void rb_class_remove_from_super_subclass https://github.com/ruby/ruby/blob/trunk/internal.h#L477
 #define RCLASS_CONST_TBL(c) (RCLASS_EXT(c)->const_tbl)
 #define RCLASS_M_TBL(c) (RCLASS(c)->m_tbl)
 #define RCLASS_IV_INDEX_TBL(c) (RCLASS_EXT(c)->iv_index_tbl)
-#define RCLASS_ORIGIN(c) (RCLASS_EXT(c)->origin)
+#define RCLASS_ORIGIN(c) (RCLASS_EXT(c)->origin_)
 #define RCLASS_REFINED_CLASS(c) (RCLASS_EXT(c)->refined_class)
 #define RCLASS_SERIAL(c) (RCLASS_EXT(c)->class_serial)
 
+#define RICLASS_IS_ORIGIN FL_USER5
+
+static inline void
+RCLASS_SET_ORIGIN(VALUE klass, VALUE origin)
+{
+    RB_OBJ_WRITE(klass, &RCLASS_ORIGIN(klass), origin);
+    if (klass != origin) FL_SET(origin, RICLASS_IS_ORIGIN);
+}
+
 static inline void
 RCLASS_M_TBL_INIT(VALUE c)
 {

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

[前][次][番号順一覧][スレッド一覧]