From 5922c954614e5947a548780bb3b894626affe6dd Mon Sep 17 00:00:00 2001 From: ko1 Date: Wed, 11 Mar 2015 09:15:20 +0000 Subject: [PATCH] * 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. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@49931 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 31 +++++++++++++++++++++++++++++++ class.c | 27 ++++++++++++++------------- gc.c | 28 ++++++++++++++++++++-------- internal.h | 13 +++++++++++-- 4 files changed, 76 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1a6f0139b7..df19eb1341 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,34 @@ +Wed Mar 11 17:03:20 2015 Koichi Sasada + + * 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 * configure.in: check also procstat_getvmmap, which is not diff --git a/class.c b/class.c index f11eb52f4d..11210682fe 100644 --- a/class.c +++ b/class.c @@ -163,21 +163,22 @@ static VALUE 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 module) 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, diff --git a/gc.c b/gc.c index 10b74b0cae..12721df6ae 100644 --- a/gc.c +++ b/gc.c @@ -1882,7 +1882,6 @@ obj_free(rb_objspace_t *objspace, VALUE obj) 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 obj) 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_types) 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_types) 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, VALUE obj) 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); diff --git a/internal.h b/internal.h index 0aa58439b5..372cf5daba 100644 --- a/internal.h +++ b/internal.h @@ -311,7 +311,7 @@ struct rb_classext_struct { */ 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_subclasses(VALUE); #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) {