Fixed bug #60139 (Anonymous functions create cycles not detected by the GC)

Zend/tests/bug60139.phpt

@@ -0,0 +1,30 @@
+--TEST--
+Bug #60139 (Anonymous functions create cycles not detected by the GC)
+--FILE--
+<?php
+class Foo {
+    public $x;
+
+    public function __construct() {
+        $this->x = function() {};
+    }
+}
+
+class Bar {
+    public $x;
+
+    public function __construct() {
+        $self = $this;
+        $this->x = function() use ($self) {};
+    }
+}
+
+gc_collect_cycles();
+new Foo;
+var_dump(gc_collect_cycles());
+new Bar;
+var_dump(gc_collect_cycles());
+?>
+--EXPECT--
+int(2)
+int(2)

Zend/zend_closures.c

@@ -376,6 +376,17 @@ static HashTable *zend_closure_get_debug_info(zval *object, int *is_temp TSRMLS_
 }
 /* }}} */
 
+static HashTable *zend_closure_get_gc(zval *obj, zval ***table, int *n TSRMLS_DC) /* {{{ */
+{
+	zend_closure *closure = (zend_closure *)zend_object_store_get_object(obj TSRMLS_CC);	
+
+	*table = closure->this_ptr ? &closure->this_ptr : NULL;
+	*n = closure->this_ptr ? 1 : 0;
+	return (closure->func.type == ZEND_USER_FUNCTION) ?
+		closure->func.op_array.static_variables : NULL;
+}
+/* }}} */
+
 /* {{{ proto Closure::__construct()
    Private constructor preventing instantiation */
 ZEND_METHOD(Closure, __construct)
@@ -425,6 +436,7 @@ void zend_register_closure_ce(TSRMLS_D) /* {{{ */
 	closure_handlers.clone_obj = zend_closure_clone;
 	closure_handlers.get_debug_info = zend_closure_get_debug_info;
 	closure_handlers.get_closure = zend_closure_get_closure;
+	closure_handlers.get_gc = zend_closure_get_gc;
 }
 /* }}} */
 

Zend/zend_gc.c

@@ -194,7 +194,7 @@ ZEND_API void gc_zobj_possible_root(zval *zv TSRMLS_DC)
 {
 	struct _store_object *obj;
 
-	if (UNEXPECTED(Z_OBJ_HT_P(zv)->get_properties == NULL ||
+	if (UNEXPECTED(Z_OBJ_HT_P(zv)->get_gc == NULL ||
 	    EG(objects_store).object_buckets == NULL)) {
 		return;
 	}
@@ -275,30 +275,27 @@ tail_call:
 	GC_ZVAL_SET_BLACK(pz);
 
 	if (Z_TYPE_P(pz) == IS_OBJECT && EG(objects_store).object_buckets) {
-		zend_object_get_properties_t get_props;
+		zend_object_get_gc_t get_gc;
 		struct _store_object *obj = &EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj;
 
 		obj->refcount++;
 		if (GC_GET_COLOR(obj->buffered) != GC_BLACK) {
 			GC_SET_BLACK(obj->buffered);
 			if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid &&
-			             (get_props = Z_OBJ_HANDLER_P(pz, get_properties)) != NULL)) {
+			             (get_gc = Z_OBJ_HANDLER_P(pz, get_gc)) != NULL)) {
-				if (get_props == zend_std_get_properties) {
+				int i, n;
-					zend_object* zobj = ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj.object));
+				zval **table;
+				HashTable *props = get_gc(pz, &table, &n TSRMLS_CC);
 
-					if (!zobj->properties) {
+				while (n > 0 && !table[n-1]) n--;
-						int i;
-						int n = zobj->ce->default_properties_count;
-
-						while (n > 0 && !zobj->properties_table[n-1]) n--;
 				for (i = 0; i < n; i++) {
-							if (zobj->properties_table[i]) {
+					if (table[i]) {
-								pz = zobj->properties_table[i];
+						pz = table[i];
 						if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {
 							pz->refcount__gc++;
 						}
 						if (GC_ZVAL_GET_COLOR(pz) != GC_BLACK) {
-									if (i == n - 1) {
+							if (!props && i == n - 1) {
 								goto tail_call;
 							} else {
 								zval_scan_black(pz TSRMLS_CC);
@@ -306,19 +303,12 @@ tail_call:
 						}
 					}
 				}
-						return;
-					} else {
-						p = zobj->properties->pListHead;
-					}
-				} else {
-					HashTable *props = get_props(pz TSRMLS_CC);
 				if (!props) {
 					return;
 				}
 				p = props->pListHead;
 			}
 		}
-		}
 	} else if (Z_TYPE_P(pz) == IS_ARRAY) {
 		if (Z_ARRVAL_P(pz) != &EG(symbol_table)) {
 			p = Z_ARRVAL_P(pz)->pListHead;
@@ -343,20 +333,18 @@ tail_call:
 static void zobj_scan_black(struct _store_object *obj, zval *pz TSRMLS_DC)
 {
 	Bucket *p;
-	zend_object_get_properties_t get_props;
+	zend_object_get_gc_t get_gc;
 
 	GC_SET_BLACK(obj->buffered);
 	if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid &&
-	             (get_props = Z_OBJ_HANDLER_P(pz, get_properties)) != NULL)) {
+	             (get_gc = Z_OBJ_HANDLER_P(pz, get_gc)) != NULL)) {
-		if (get_props == zend_std_get_properties) {
+		int i, n;
-			zend_object* zobj = ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj.object));
+		zval **table;
+		HashTable *props = get_gc(pz, &table, &n TSRMLS_CC);
 
-			if (!zobj->properties) {
+		for (i = 0; i < n; i++) {
-				int i;
+			if (table[i]) {
-
+				pz = table[i];
-				for (i = 0; i < zobj->ce->default_properties_count; i++) {
-					if (zobj->properties_table[i]) {
-						pz = zobj->properties_table[i];
 				if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {
 					pz->refcount__gc++;
 				}
@@ -365,17 +353,10 @@ static void zobj_scan_black(struct _store_object *obj, zval *pz TSRMLS_DC)
 				}
 			}
 		}
-				return;
-			} else {
-				p = zobj->properties->pListHead;
-			}
-		} else {
-			HashTable *props = get_props(pz TSRMLS_CC);
 		if (!props) {
 			return;
 		}
 		p = props->pListHead;
-		}
 		while (p != NULL) {
 			pz = *(zval**)p->pData;
 			if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {
@@ -400,7 +381,7 @@ tail_call:
 		GC_ZVAL_SET_COLOR(pz, GC_GREY);
 
 		if (Z_TYPE_P(pz) == IS_OBJECT && EG(objects_store).object_buckets) {
-			zend_object_get_properties_t get_props;
+			zend_object_get_gc_t get_gc;
 			struct _store_object *obj = &EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj;
 
 			obj->refcount--;
@@ -408,41 +389,31 @@ tail_call:
 				GC_BENCH_INC(zobj_marked_grey);
 				GC_SET_COLOR(obj->buffered, GC_GREY);
 				if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid &&
-				             (get_props = Z_OBJ_HANDLER_P(pz, get_properties)) != NULL)) {
+				             (get_gc = Z_OBJ_HANDLER_P(pz, get_gc)) != NULL)) {
-					if (get_props == zend_std_get_properties) {
+					int i, n;
-						zend_object* zobj = ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj.object));
+					zval **table;
+					HashTable *props = get_gc(pz, &table, &n TSRMLS_CC);
 
-						if (!zobj->properties) {
+					while (n > 0 && !table[n-1]) n--;
-							int i;
-							int n = zobj->ce->default_properties_count;
-
-							while (n > 0 && !zobj->properties_table[n-1]) n--;
 					for (i = 0; i < n; i++) {
-								if (zobj->properties_table[i]) {
+						if (table[i]) {
-									pz = zobj->properties_table[i];
+							pz = table[i];
 							if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {
 								pz->refcount__gc--;
 							}
-									if (i == n - 1) {
+							if (!props && i == n - 1) {
 								goto tail_call;
 							} else {
 								zval_mark_grey(pz TSRMLS_CC);
 							}
 						}
 					}
-							return;
-						} else {
-							p = zobj->properties->pListHead;
-						}
-					} else {
-						HashTable *props = get_props(pz TSRMLS_CC);
 					if (!props) {
 						return;
 					}
 					p = props->pListHead;
 				}
 			}
-			}
 		} else if (Z_TYPE_P(pz) == IS_ARRAY) {
 			if (Z_ARRVAL_P(pz) == &EG(symbol_table)) {
 				GC_ZVAL_SET_BLACK(pz);
@@ -468,39 +439,30 @@ tail_call:
 static void zobj_mark_grey(struct _store_object *obj, zval *pz TSRMLS_DC)
 {
 	Bucket *p;
-	zend_object_get_properties_t get_props;
+	zend_object_get_gc_t get_gc;
 
 	if (GC_GET_COLOR(obj->buffered) != GC_GREY) {
 		GC_BENCH_INC(zobj_marked_grey);
 		GC_SET_COLOR(obj->buffered, GC_GREY);
 		if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid &&
-		             (get_props = Z_OBJ_HANDLER_P(pz, get_properties)) != NULL)) {
+		             (get_gc = Z_OBJ_HANDLER_P(pz, get_gc)) != NULL)) {
-			if (get_props == zend_std_get_properties) {
+			int i, n;
-				zend_object* zobj = ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj.object));
+			zval **table;
+			HashTable *props = get_gc(pz, &table, &n TSRMLS_CC);
 
-				if (!zobj->properties) {
+			for (i = 0; i < n; i++) {
-					int i;
+				if (table[i]) {
-
+					pz = table[i];
-					for (i = 0; i < zobj->ce->default_properties_count; i++) {
-						if (zobj->properties_table[i]) {
-							pz = zobj->properties_table[i];
 					if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {
 						pz->refcount__gc--;
 					}
 					zval_mark_grey(pz TSRMLS_CC);
 				}
 			}
-					return;
-				} else {
-					p = zobj->properties->pListHead;
-				}
-			} else {
-				HashTable *props = get_props(pz TSRMLS_CC);
 			if (!props) {
 				return;
 			}
 			p = props->pListHead;
-			}
 			while (p != NULL) {
 				pz = *(zval**)p->pData;
 				if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {
@@ -558,7 +520,7 @@ tail_call:
 		} else {
 			GC_ZVAL_SET_COLOR(pz, GC_WHITE);
 			if (Z_TYPE_P(pz) == IS_OBJECT && EG(objects_store).object_buckets) {
-				zend_object_get_properties_t get_props;
+				zend_object_get_gc_t get_gc;
 				struct _store_object *obj = &EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj;
 
 				if (GC_GET_COLOR(obj->buffered) == GC_GREY) {
@@ -567,31 +529,22 @@ tail_call:
 					} else {
 						GC_SET_COLOR(obj->buffered, GC_WHITE);
 						if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid &&
-						             (get_props = Z_OBJ_HANDLER_P(pz, get_properties)) != NULL)) {
+						             (get_gc = Z_OBJ_HANDLER_P(pz, get_gc)) != NULL)) {
-							if (get_props == zend_std_get_properties) {
+							int i, n;
-								zend_object* zobj = ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj.object));
+							zval **table;
+							HashTable *props = get_gc(pz, &table, &n TSRMLS_CC);
 
-								if (!zobj->properties) {
+							while (n > 0 && !table[n-1]) n--;
-									int i;
-									int n = zobj->ce->default_properties_count;
-
-									while (n > 0 && !zobj->properties_table[n-1]) n--;
 							for (i = 0; i < n; i++) {
-										if (zobj->properties_table[i]) {
+								if (table[i]) {
-											pz = zobj->properties_table[i];
+									pz = table[i];
-											if (i == n - 1) {
+									if (!props && i == n - 1) {
 										goto tail_call;
 									} else {
 										zval_scan(pz TSRMLS_CC);
 									}
 								}
 							}
-									return;
-								} else {
-									p = zobj->properties->pListHead;
-								}
-							} else {
-								HashTable *props = get_props(pz TSRMLS_CC);
 							if (!props) {
 								return;
 							}
@@ -599,7 +552,6 @@ tail_call:
 						}
 					}
 				}
-				}
 			} else if (Z_TYPE_P(pz) == IS_ARRAY) {
 				if (Z_ARRVAL_P(pz) == &EG(symbol_table)) {
 					GC_ZVAL_SET_BLACK(pz);
@@ -623,7 +575,7 @@ tail_call:
 static void zobj_scan(zval *pz TSRMLS_DC)
 {
 	Bucket *p;
-	zend_object_get_properties_t get_props;
+	zend_object_get_gc_t get_gc;
 
 	if (EG(objects_store).object_buckets) {
 		struct _store_object *obj = &EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj;
@@ -634,30 +586,21 @@ static void zobj_scan(zval *pz TSRMLS_DC)
 			} else {
 				GC_SET_COLOR(obj->buffered, GC_WHITE);
 				if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid &&
-				             (get_props = Z_OBJ_HANDLER_P(pz, get_properties)) != NULL)) {
+				             (get_gc = Z_OBJ_HANDLER_P(pz, get_gc)) != NULL)) {
-					if (get_props == zend_std_get_properties) {
+					int i, n;
-						zend_object* zobj = ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj.object));
+					zval **table;
+					HashTable *props = get_gc(pz, &table, &n TSRMLS_CC);
 
-						if (!zobj->properties) {
+					for (i = 0; i < n; i++) {
-							int i;
+						if (table[i]) {
-
+							pz = table[i];
-							for (i = 0; i < zobj->ce->default_properties_count; i++) {
-								if (zobj->properties_table[i]) {
-									pz = zobj->properties_table[i];
 							zval_scan(pz TSRMLS_CC);
                     	}
 					}
-							return;
-						} else {
-							p = zobj->properties->pListHead;
-						}
-					} else {
-						HashTable *props = get_props(pz TSRMLS_CC);
 					if (!props) {
 						return;
 					}
 					p = props->pListHead;
-					}
 					while (p != NULL) {
 						zval_scan(*(zval**)p->pData TSRMLS_CC);
 						p = p->pListNext;
@@ -697,54 +640,46 @@ tail_call:
 		GC_ZVAL_SET_BLACK(pz);
 
 		if (Z_TYPE_P(pz) == IS_OBJECT && EG(objects_store).object_buckets) {
-			zend_object_get_properties_t get_props;
+			zend_object_get_gc_t get_gc;
 			struct _store_object *obj = &EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj;
 
 			if (obj->buffered == (gc_root_buffer*)GC_WHITE) {
 				GC_SET_BLACK(obj->buffered);
 
 				if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid &&
-				             (get_props = Z_OBJ_HANDLER_P(pz, get_properties)) != NULL)) {
+				             (get_gc = Z_OBJ_HANDLER_P(pz, get_gc)) != NULL)) {
-					if (get_props == zend_std_get_properties) {
+					int i, n;
-						zend_object* zobj = ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj.object));
+					zval **table, *zv;
-
+					HashTable *props = get_gc(pz, &table, &n TSRMLS_CC);
-						if (!zobj->properties) {
-							int i;
-							int n = zobj->ce->default_properties_count;
-
-							while (n > 0 && !zobj->properties_table[n-1]) n--;
 
+					if (!props) {
 						/* 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;
-
-							for (i = 0; i < n; i++) {
-								if (zobj->properties_table[i]) {
-									pz = zobj->properties_table[i];
-									if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {
-										pz->refcount__gc++;
 					}
-									if (i == n - 1) {
+
+					while (n > 0 && !table[n-1]) n--;
+					for (i = 0; i < n; i++) {
+						if (table[i]) {
+							zv = table[i];
+							if (Z_TYPE_P(zv) != IS_ARRAY || Z_ARRVAL_P(zv) != &EG(symbol_table)) {
+								zv->refcount__gc++;
+							}
+							if (!props && i == n - 1) {
+								pz = zv;
 								goto tail_call;
 							} else {
-										zval_collect_white(pz TSRMLS_CC);
+								zval_collect_white(zv TSRMLS_CC);
 							}
 						}
 					}
-							return;
-						} else {
-							p = zobj->properties->pListHead;
-						}
-					} else {
-						HashTable *props = get_props(pz TSRMLS_CC);
 					if (!props) {
 						return;
 					}
 					p = props->pListHead;
 				}
 			}
-			}
 		} else {
 			if (Z_TYPE_P(pz) == IS_ARRAY) {
 				p = Z_ARRVAL_P(pz)->pListHead;
@@ -776,40 +711,31 @@ static void zobj_collect_white(zval *pz TSRMLS_DC)
 	Bucket *p;
 
 	if (EG(objects_store).object_buckets) {
-		zend_object_get_properties_t get_props;
+		zend_object_get_gc_t get_gc;
 		struct _store_object *obj = &EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj;
 
 		if (obj->buffered == (gc_root_buffer*)GC_WHITE) {
 			GC_SET_BLACK(obj->buffered);
 
 			if (EXPECTED(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].valid &&
-			             (get_props = Z_OBJ_HANDLER_P(pz, get_properties)) != NULL)) {
+			             (get_gc = Z_OBJ_HANDLER_P(pz, get_gc)) != NULL)) {
-				if (get_props == zend_std_get_properties) {
+				int i, n;
-					zend_object* zobj = ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(pz)].bucket.obj.object));
+				zval **table;
+				HashTable *props = get_gc(pz, &table, &n TSRMLS_CC);
 
-					if (!zobj->properties) {
+				for (i = 0; i < n; i++) {
-						int i;
+					if (table[i]) {
-
+						pz = table[i];
-						for (i = 0; i < zobj->ce->default_properties_count; i++) {
-							if (zobj->properties_table[i]) {
-								pz = zobj->properties_table[i];
 						if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {
 							pz->refcount__gc++;
 						}
 						zval_collect_white(pz TSRMLS_CC);
 					}
 				}
-						return;
-					} else {
-						p = zobj->properties->pListHead;
-					}
-				} else {
-					HashTable *props = get_props(pz TSRMLS_CC);
 				if (!props) {
 					return;
 				}
 				p = props->pListHead;
-				}
 				while (p != NULL) {
 					pz = *(zval**)p->pData;
 					if (Z_TYPE_P(pz) != IS_ARRAY || Z_ARRVAL_P(pz) != &EG(symbol_table)) {

Zend/zend_object_handlers.c

@@ -102,6 +102,28 @@ ZEND_API HashTable *zend_std_get_properties(zval *object TSRMLS_DC) /* {{{ */
 }
 /* }}} */
 
+ZEND_API HashTable *zend_std_get_gc(zval *object, zval ***table, int *n TSRMLS_DC) /* {{{ */
+{
+	if (Z_OBJ_HANDLER_P(object, get_properties) != zend_std_get_properties) {
+		*table = NULL;
+		*n = 0;
+		return Z_OBJ_HANDLER_P(object, get_properties)(object TSRMLS_CC);
+	} else {
+		zend_object *zobj = Z_OBJ_P(object);
+
+		if (zobj->properties) {
+			*table = NULL;
+			*n = 0;
+			return zobj->properties;
+		} else {
+			*table = zobj->properties_table;
+			*n = zobj->ce->default_properties_count;
+			return NULL;
+		}
+	}
+}
+/* }}} */
+
 ZEND_API HashTable *zend_std_get_debug_info(zval *object, int *is_temp TSRMLS_DC) /* {{{ */
 {
 	*is_temp = 0;
@@ -1584,6 +1606,7 @@ ZEND_API zend_object_handlers std_object_handlers = {
 	NULL,									/* count_elements */
 	NULL,									/* get_debug_info */
 	zend_std_get_closure,					/* get_closure */
+	zend_std_get_gc,						/* get_gc */
 };
 
 /*

Zend/zend_object_handlers.h

@@ -111,6 +111,8 @@ typedef int (*zend_object_count_elements_t)(zval *object, long *count TSRMLS_DC)
 
 typedef int (*zend_object_get_closure_t)(zval *obj, zend_class_entry **ce_ptr, union _zend_function **fptr_ptr, zval **zobj_ptr TSRMLS_DC);
 
+typedef HashTable *(*zend_object_get_gc_t)(zval *object, zval ***table, int *n TSRMLS_DC);
+
 struct _zend_object_handlers {
 	/* general object functions */
 	zend_object_add_ref_t					add_ref;
@@ -139,6 +141,7 @@ struct _zend_object_handlers {
 	zend_object_count_elements_t			count_elements;
 	zend_object_get_debug_info_t			get_debug_info;
 	zend_object_get_closure_t				get_closure;
+	zend_object_get_gc_t					get_gc;
 };
 
 extern ZEND_API zend_object_handlers std_object_handlers;