From: Nikita Popov <nikita.ppv@gmail.com>
Date: Wed, 28 Aug 2019 10:15:28 +0000 (+0200)
Subject: Fixed bug #78335
X-Git-Tag: php-7.4.0RC1~30
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ec9a96dc60c3fd2cccf5b21033a0e87807366bdb;p=php

Fixed bug #78335

Destroy static properties and variables prior to the final GC run,
as they may hold GC roots.
---

diff --git a/NEWS b/NEWS
index 9a24ba158c..8c31ec8de0 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,8 @@ PHP                                                                        NEWS
     (cmb)
   . Fixed bug #78454 (Consecutive numeric separators cause OOM error).
     (Theodore Brown)
+  . Fixed bug #78335 (Static properties/variables containing cycles report as
+    leak). (Nikita)
 
 - FPM:
   . Fixed bug #78334 (fpm log prefix message includes wrong stdout/stderr
diff --git a/Zend/tests/bug78335.phpt b/Zend/tests/bug78335.phpt
new file mode 100644
index 0000000000..68e885885e
--- /dev/null
+++ b/Zend/tests/bug78335.phpt
@@ -0,0 +1,28 @@
+--TEST--
+Bug #78335: Static properties/variables containing cycles report as leak
+--FILE--
+<?php
+
+class Test {
+    public static $test;
+
+    public static function method() {
+        static $foo;
+        $foo = [&$foo];
+    }
+}
+
+function test() {
+    static $foo;
+    $foo = [&$foo];
+}
+
+$foo = [&$foo];
+Test::$test = $foo;
+test();
+Test::method();
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c
index 161390e22a..ac2ee8ed7f 100644
--- a/Zend/zend_execute_API.c
+++ b/Zend/zend_execute_API.c
@@ -270,9 +270,54 @@ void shutdown_executor(void) /* {{{ */
 		zend_close_rsrc_list(&EG(regular_list));
 	} zend_end_try();
 
+	/* No PHP callback functions should be called after this point. */
+	EG(active) = 0;
+
 	if (!fast_shutdown) {
 		zend_hash_graceful_reverse_destroy(&EG(symbol_table));
 
+		/* Release static properties and static variables prior to the final GC run,
+		 * as they may hold GC roots. */
+		ZEND_HASH_REVERSE_FOREACH_VAL(EG(function_table), zv) {
+			zend_op_array *op_array = Z_PTR_P(zv);
+			if (op_array->type == ZEND_INTERNAL_FUNCTION) {
+				break;
+			}
+			if (op_array->static_variables) {
+				HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
+				if (ht) {
+					ZEND_ASSERT(GC_REFCOUNT(ht) == 1);
+					zend_array_destroy(ht);
+					ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
+				}
+			}
+		} ZEND_HASH_FOREACH_END();
+		ZEND_HASH_REVERSE_FOREACH_VAL(EG(class_table), zv) {
+			zend_class_entry *ce = Z_PTR_P(zv);
+			if (ce->type == ZEND_INTERNAL_CLASS) {
+				break;
+			}
+			if (ce->default_static_members_count) {
+				zend_cleanup_internal_class_data(ce);
+			}
+			if (ce->ce_flags & ZEND_HAS_STATIC_IN_METHODS) {
+				zend_op_array *op_array;
+					ZEND_HASH_FOREACH_PTR(&ce->function_table, op_array) {
+					if (op_array->type == ZEND_USER_FUNCTION) {
+						if (op_array->static_variables) {
+							HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
+							if (ht) {
+								if (GC_DELREF(ht) == 0) {
+									zend_array_destroy(ht);
+								}
+								ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
+							}
+						}
+					}
+				} ZEND_HASH_FOREACH_END();
+			}
+		} ZEND_HASH_FOREACH_END();
+
 #if ZEND_DEBUG
 		if (gc_enabled() && !CG(unclean_shutdown)) {
 			gc_collect_cycles();
@@ -284,10 +329,6 @@ void shutdown_executor(void) /* {{{ */
 
 	zend_weakrefs_shutdown();
 
-	/* All resources and objects are destroyed. */
-	/* No PHP callback functions may be called after this point. */
-	EG(active) = 0;
-
 	zend_try {
 		zend_llist_apply(&zend_extensions, (llist_apply_func_t) zend_extension_deactivator);
 	} zend_end_try();
@@ -348,23 +389,6 @@ void shutdown_executor(void) /* {{{ */
 				zend_string_release_ex(key, 0);
 			} ZEND_HASH_FOREACH_END_DEL();
 
-			/* Cleanup preloaded immutable functions */
-			ZEND_HASH_REVERSE_FOREACH_VAL(EG(function_table), zv) {
-				zend_op_array *op_array = Z_PTR_P(zv);
-				if (op_array->type == ZEND_INTERNAL_FUNCTION) {
-					break;
-				}
-				ZEND_ASSERT(op_array->fn_flags & ZEND_ACC_IMMUTABLE);
-				if (op_array->static_variables) {
-					HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
-					if (ht) {
-						ZEND_ASSERT(GC_REFCOUNT(ht) == 1);
-						zend_array_destroy(ht);
-						ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
-					}
-				}
-			} ZEND_HASH_FOREACH_END();
-
 			ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(class_table), key, zv) {
 				if (_idx == EG(persistent_classes_count)) {
 					break;
@@ -372,33 +396,6 @@ void shutdown_executor(void) /* {{{ */
 				destroy_zend_class(zv);
 				zend_string_release_ex(key, 0);
 			} ZEND_HASH_FOREACH_END_DEL();
-
-			/* Cleanup preloaded immutable classes */
-			ZEND_HASH_REVERSE_FOREACH_VAL(EG(class_table), zv) {
-				zend_class_entry *ce = Z_PTR_P(zv);
-				if (ce->type == ZEND_INTERNAL_CLASS) {
-					break;
-				}
-				ZEND_ASSERT(ce->ce_flags & ZEND_ACC_IMMUTABLE);
-				if (ce->default_static_members_count) {
-					zend_cleanup_internal_class_data(ce);
-				}
-				if (ce->ce_flags & ZEND_HAS_STATIC_IN_METHODS) {
-					zend_op_array *op_array;
-						ZEND_HASH_FOREACH_PTR(&ce->function_table, op_array) {
-						if (op_array->type == ZEND_USER_FUNCTION) {
-							if (op_array->static_variables) {
-								HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
-								if (ht) {
-									ZEND_ASSERT(GC_REFCOUNT(ht) == 1);
-									zend_array_destroy(ht);
-									ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
-								}
-							}
-						}
-					} ZEND_HASH_FOREACH_END();
-				}
-			} ZEND_HASH_FOREACH_END();
 		}
 
 		zend_cleanup_internal_classes();