cvs: ZendEngine2 / zend_gc.c zend_gc.h /tests gc_028.phpt

http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_gc.c?r1=1.7&r2=1.8&diff_format=u
Index: ZendEngine2/zend_gc.c
diff -u ZendEngine2/zend_gc.c:1.7 ZendEngine2/zend_gc.c:1.8
--- ZendEngine2/zend_gc.c:1.7	Thu Feb 21 10:42:22 2008
+++ ZendEngine2/zend_gc.c	Fri Mar 14 13:21:21 2008
@@ -17,7 +17,7 @@
    +----------------------------------------------------------------------+
 */
 
-/* $Id: zend_gc.c,v 1.7 2008/02/21 10:42:22 dmitry Exp $ */
+/* $Id: zend_gc.c,v 1.8 2008/03/14 13:21:21 dmitry Exp $ */
 
 #include "zend.h"
 #include "zend_API.h"
@@ -55,6 +55,7 @@
 	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_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_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 @@
 			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 @@
 
 		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 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 @@
 	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 @@
 		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;
http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_gc.h?r1=1.4&r2=1.5&diff_format=u
Index: ZendEngine2/zend_gc.h
diff -u ZendEngine2/zend_gc.h:1.4 ZendEngine2/zend_gc.h:1.5
--- ZendEngine2/zend_gc.h:1.4	Tue Jan 29 09:59:53 2008
+++ ZendEngine2/zend_gc.h	Fri Mar 14 13:21:21 2008
@@ -17,7 +17,7 @@
    +----------------------------------------------------------------------+
 */
 
-/* $Id: zend_gc.h,v 1.4 2008/01/29 09:59:53 dmitry Exp $ */
+/* $Id: zend_gc.h,v 1.5 2008/03/14 13:21:21 dmitry Exp $ */
 
 #ifndef ZEND_GC_H
 #define ZEND_GC_H
@@ -105,6 +105,7 @@
 	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 @@
 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 @@
 
 #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_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);
 	}
 }
 
http://cvs.php.net/viewvc.cgi/ZendEngine2/tests/gc_028.phpt?r1=1.1&r2=1.2&diff_format=u
Index: ZendEngine2/tests/gc_028.phpt
diff -u /dev/null ZendEngine2/tests/gc_028.phpt:1.2
--- /dev/null	Fri Mar 14 13:21:21 2008
+++ ZendEngine2/tests/gc_028.phpt	Fri Mar 14 13:21:21 2008
@@ -0,0 +1,31 @@
+--TEST--
+GC 028: GC and destructors
+--FILE--
+<?php
+class Foo {
+	public $bar;
+	function __destruct() {
+		if ($this->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)