From: Dmitry Stogov Date: Fri, 14 Mar 2008 13:21:04 +0000 (+0000) Subject: Fixed GC bug X-Git-Tag: BEFORE_NEW_PARAMETER_PARSE~591 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e1a28ee945c5954b84fdfed640a520d81fc3245e;p=php Fixed GC bug --- diff --git a/Zend/tests/gc_028.phpt b/Zend/tests/gc_028.phpt new file mode 100644 index 0000000000..6f330cb60b --- /dev/null +++ b/Zend/tests/gc_028.phpt @@ -0,0 +1,31 @@ +--TEST-- +GC 028: GC and destructors +--FILE-- +bar !== null) { + unset($this->bar); + } + } +} +class Bar { + public $foo; + function __destruct() { + if ($this->foo !== null) { + unset($this->foo); + } + } + +} +$foo = new Foo(); +$bar = new Bar(); +$foo->bar = $bar; +$bar->foo = $foo; +unset($foo); +unset($bar); +var_dump(gc_collect_cycles()); +?> +--EXPECT-- +int(2) diff --git a/Zend/zend_gc.c b/Zend/zend_gc.c index 72cc457024..30df9aa64c 100644 --- a/Zend/zend_gc.c +++ b/Zend/zend_gc.c @@ -55,6 +55,7 @@ static void gc_globals_ctor_ex(zend_gc_globals *gc_globals TSRMLS_DC) gc_globals->roots.prev = NULL; gc_globals->unused = NULL; gc_globals->zval_to_free = NULL; + gc_globals->free_list = NULL; gc_globals->gc_runs = 0; gc_globals->collected = 0; @@ -136,6 +137,19 @@ ZEND_API void gc_init(TSRMLS_D) ZEND_API void gc_zval_possible_root(zval *zv TSRMLS_DC) { + if (UNEXPECTED(GC_G(free_list) != NULL && + GC_ZVAL_ADDRESS(zv) != NULL && + GC_ZVAL_GET_COLOR(zv) == GC_BLACK)) { + zval_gc_info **p = &GC_G(free_list); + + while (*p != NULL) { + if (*p == (zval_gc_info*)zv) { + return; + } + p = &(*p)->u.next; + } + } + if (zv->type == IS_OBJECT) { GC_ZOBJ_CHECK_POSSIBLE_ROOT(zv); return; @@ -235,6 +249,28 @@ ZEND_API void gc_zobj_possible_root(zval *zv TSRMLS_DC) } } +ZEND_API void _gc_remove_zval_from_buffer(zval *zv) +{ + gc_root_buffer* root_buffer = GC_ADDRESS(((zval_gc_info*)zv)->u.buffered); + TSRMLS_FETCH(); + + if (UNEXPECTED(GC_G(free_list) != NULL && + GC_ZVAL_GET_COLOR(zv) == GC_BLACK)) { + zval_gc_info **p = &GC_G(free_list); + + while (*p != NULL) { + if (*p == (zval_gc_info*)zv) { + *p = (*p)->u.next; + return; + } + p = &(*p)->u.next; + } + } + GC_BENCH_INC(zval_remove_from_buffer); + GC_REMOVE_FROM_BUFFER(root_buffer); + ((zval_gc_info*)zv)->u.buffered = NULL; +} + static void zobj_scan_black(struct _store_object *obj, zval *pz TSRMLS_DC) { GC_SET_BLACK(obj->buffered); @@ -371,7 +407,6 @@ static int zval_scan(zval *pz TSRMLS_DC) zval_scan_black(pz TSRMLS_CC); } else { GC_ZVAL_SET_COLOR(pz, GC_WHITE); - if (Z_TYPE_P(pz) == IS_OBJECT) { zobj_scan(pz TSRMLS_CC); } else if (Z_TYPE_P(pz) == IS_ARRAY) { @@ -435,17 +470,22 @@ static void zval_collect_white(zval *pz TSRMLS_DC) if (Z_TYPE_P(pz) == IS_OBJECT) { zobj_collect_white(pz TSRMLS_CC); + if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid && + Z_OBJ_HANDLER_P(pz, get_properties) != NULL)) { + Z_OBJPROP_P(pz)->pDestructor = NULL; + } } else { if (Z_TYPE_P(pz) == IS_ARRAY) { - if (Z_ARRVAL_P(pz) == &EG(symbol_table)) { - return; - } +// if (Z_ARRVAL_P(pz) == &EG(symbol_table)) { +// return; +// } zend_hash_apply(Z_ARRVAL_P(pz), (apply_func_t) children_collect_white TSRMLS_CC); + Z_ARRVAL_P(pz)->pDestructor = NULL; } - /* restore refcount */ - pz->refcount__gc++; } + /* restore refcount and put into list to free */ + pz->refcount__gc++; ((zval_gc_info*)pz)->u.next = GC_G(zval_to_free); GC_G(zval_to_free) = (zval_gc_info*)pz; } @@ -453,6 +493,9 @@ static void zval_collect_white(zval *pz TSRMLS_DC) static int children_collect_white(zval **pz TSRMLS_DC) { + if (Z_TYPE_PP(pz) != IS_ARRAY || Z_ARRVAL_PP(pz) != &EG(symbol_table)) { + (*pz)->refcount__gc++; + } zval_collect_white(*pz TSRMLS_CC); return 0; } @@ -486,7 +529,7 @@ ZEND_API int gc_collect_cycles(TSRMLS_D) int count = 0; if (GC_G(roots).next != &GC_G(roots)) { - zval_gc_info *p, *q; + zval_gc_info *p, *q, *orig_free_list; if (GC_G(gc_active)) { return 0; @@ -497,35 +540,53 @@ ZEND_API int gc_collect_cycles(TSRMLS_D) gc_mark_roots(TSRMLS_C); gc_scan_roots(TSRMLS_C); gc_collect_roots(TSRMLS_C); - GC_G(gc_active) = 0; - p = GC_G(zval_to_free); + orig_free_list = GC_G(free_list); + p = GC_G(free_list) = GC_G(zval_to_free); GC_G(zval_to_free) = NULL; + GC_G(gc_active) = 0; + + /* First call destructors */ + while (p) { + if (Z_TYPE(p->z) == IS_OBJECT) { + if (EG(objects_store).object_buckets && + EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].valid && + EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.refcount <= 0 && + EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.dtor && + !EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].destructor_called) { + + EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].destructor_called = 1; + zend_try { + EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.dtor(EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.object, Z_OBJ_HANDLE(p->z) TSRMLS_CC); + } zend_end_try(); + } + } + p = p->u.next; + } + + p = GC_G(free_list); while (p) { q = p->u.next; if (Z_TYPE(p->z) == IS_OBJECT) { if (EG(objects_store).object_buckets && EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].valid && EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.refcount <= 0) { - if (EXPECTED(Z_OBJ_HANDLER(p->z, get_properties) != NULL)) { - GC_G(gc_active) = 1; - Z_OBJPROP(p->z)->pDestructor = NULL; - GC_G(gc_active) = 0; - } EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.refcount = 1; - zend_objects_store_del_ref_by_handle(Z_OBJ_HANDLE(p->z) TSRMLS_CC); + zend_try { + zend_objects_store_del_ref_by_handle(Z_OBJ_HANDLE(p->z) TSRMLS_CC); + } zend_end_try(); } } else { - if (Z_TYPE(p->z) == IS_ARRAY) { - Z_ARRVAL(p->z)->pDestructor = NULL; - } - zval_dtor(&p->z); + zend_try { + zval_dtor(&p->z); + } zend_end_try(); } FREE_ZVAL_EX(&p->z); p = q; count++; } GC_G(collected) += count; + GC_G(free_list) = orig_free_list; } return count; diff --git a/Zend/zend_gc.h b/Zend/zend_gc.h index 51617b77a9..16d43df057 100644 --- a/Zend/zend_gc.h +++ b/Zend/zend_gc.h @@ -105,6 +105,7 @@ typedef struct _zend_gc_globals { gc_root_buffer *unused; /* list of unused buffers */ zval_gc_info *zval_to_free; /* temporaryt list of zvals to free */ + zval_gc_info *free_list; zend_uint gc_runs; zend_uint collected; @@ -138,6 +139,7 @@ BEGIN_EXTERN_C() ZEND_API int gc_collect_cycles(TSRMLS_D); ZEND_API void gc_zval_possible_root(zval *zv TSRMLS_DC); ZEND_API void gc_zobj_possible_root(zval *zv TSRMLS_DC); +ZEND_API void _gc_remove_zval_from_buffer(zval *zv); ZEND_API void gc_globals_ctor(TSRMLS_D); ZEND_API void gc_globals_dtor(TSRMLS_D); ZEND_API void gc_init(TSRMLS_D); @@ -163,7 +165,7 @@ END_EXTERN_C() #define GC_REMOVE_ZOBJ_FROM_BUFFER(obj) \ do { \ - if (GC_ADDRESS((obj)->buffered)) { \ + if (GC_ADDRESS((obj)->buffered) && !GC_G(gc_active)) { \ GC_BENCH_INC(zobj_remove_from_buffer); \ GC_REMOVE_FROM_BUFFER(GC_ADDRESS((obj)->buffered)); \ (obj)->buffered = NULL; \ @@ -188,15 +190,8 @@ static zend_always_inline void gc_remove_from_buffer(gc_root_buffer *root TSRMLS static zend_always_inline void gc_remove_zval_from_buffer(zval* z) { - gc_root_buffer* root_buffer; - - root_buffer = GC_ADDRESS(((zval_gc_info*)z)->u.buffered); - if (root_buffer) { - TSRMLS_FETCH(); - - GC_BENCH_INC(zval_remove_from_buffer); - GC_REMOVE_FROM_BUFFER(root_buffer); - ((zval_gc_info*)z)->u.buffered = NULL; + if (GC_ADDRESS(((zval_gc_info*)z)->u.buffered)) { + _gc_remove_zval_from_buffer(z); } }