Reference dynamic functions through dynamic_defs

Currently, dynamically declared functions and closures are inserted
into the function table under a runtime definition key, and then later
possibly renamed. When opcache is not used and a file containing a
closure is repeatedly included, this leads to a very large memory leak,
as the no longer needed closure declarations will never be freed
(https://bugs.php.net/bug.php?id=76982).

With this patch, dynamic functions are instead stored in a
dynamic_func_defs member on the op_array, which opcodes reference
by index. When the parent op_array is destroyed, the dynamic_func_defs
it contains are also destroyed (unless they are stilled used elsewhere,
e.g. because they have been bound, or are used by a live closure). This
resolves the fundamental part of the leak, though doesn't completely
fix it yet due to some arena allocations.

The main non-obvious change here is to static variable handling:
We can't destroy static_variables_ptr in destroy_op_array, as e.g.
that would clear the static variables in a dynamic function when
the op_array containing it is destroyed. Static variable destruction
is separated out for this reason (we already do static variable
destruction separately for normal functions, so we only need to
handle main scripts).

Closes GH-5595.

4 files changed, 76 insertions, 28 deletions

diff --git a/ext/opcache/zend_accelerator_util_funcs.c b/ext/opcache/zend_accelerator_util_funcs.c
index 327e8a4d55..ad456c1bcb 100644
--- a/ext/opcache/zend_accelerator_util_funcs.c
+++ b/ext/opcache/zend_accelerator_util_funcs.c
@@ -138,23 +138,9 @@ static void zend_accel_function_hash_copy(HashTable *target, HashTable *source)
 		ZEND_ASSERT(p->key);
 		t = zend_hash_find_ex(target, p->key, 1);
 		if (UNEXPECTED(t != NULL)) {
-			if (EXPECTED(ZSTR_LEN(p->key) > 0) && EXPECTED(ZSTR_VAL(p->key)[0] == 0)) {
-				/* Runtime definition key. There are two circumstances under which the key can
-				 * already be defined:
-				 *  1. The file has been re-included without being changed in the meantime. In
-				 *     this case we can keep the old value, because we know that the definition
-				 *     hasn't changed.
-				 *  2. The file has been changed in the meantime, but the RTD key ends up colliding.
-				 *     This would be a bug.
-				 * As we can't distinguish these cases, we assume that it is 1. and keep the old
-				 * value. */
-				continue;
-			} else {
-				goto failure;
-			}
-		} else {
-			_zend_hash_append_ptr_ex(target, p->key, Z_PTR(p->val), 1);
+			goto failure;
 		}
+		_zend_hash_append_ptr_ex(target, p->key, Z_PTR(p->val), 1);
 	}
 	target->nInternalPointer = 0;
 	return;
@@ -190,7 +176,15 @@ static void zend_accel_class_hash_copy(HashTable *target, HashTable *source)
 		t = zend_hash_find_ex(target, p->key, 1);
 		if (UNEXPECTED(t != NULL)) {
 			if (EXPECTED(ZSTR_LEN(p->key) > 0) && EXPECTED(ZSTR_VAL(p->key)[0] == 0)) {
-				/* See comment in zend_accel_function_hash_copy(). */
+				/* Runtime definition key. There are two circumstances under which the key can
+				 * already be defined:
+				 *  1. The file has been re-included without being changed in the meantime. In
+				 *     this case we can keep the old value, because we know that the definition
+				 *     hasn't changed.
+				 *  2. The file has been changed in the meantime, but the RTD key ends up colliding.
+				 *     This would be a bug.
+				 * As we can't distinguish these cases, we assume that it is 1. and keep the old
+				 * value. */
 				continue;
 			} else if (UNEXPECTED(!ZCG(accel_directives).ignore_dups)) {
 				zend_class_entry *ce1 = Z_PTR(p->val);
diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c
index d89c462df5..1021992b3a 100644
--- a/ext/opcache/zend_file_cache.c
+++ b/ext/opcache/zend_file_cache.c
@@ -606,6 +606,20 @@ static void zend_file_cache_serialize_op_array(zend_op_array            *op_arra
 			}
 		}
 
+		if (op_array->num_dynamic_func_defs) {
+			zend_op_array **defs;
+			SERIALIZE_PTR(op_array->dynamic_func_defs);
+			defs = op_array->dynamic_func_defs;
+			UNSERIALIZE_PTR(defs);
+			for (uint32_t i = 0; i < op_array->num_dynamic_func_defs; i++) {
+				zend_op_array *def;
+				SERIALIZE_PTR(defs[i]);
+				def = defs[i];
+				UNSERIALIZE_PTR(def);
+				zend_file_cache_serialize_op_array(def, script, info, buf);
+			}
+		}
+
 		SERIALIZE_STR(op_array->function_name);
 		SERIALIZE_STR(op_array->filename);
 		SERIALIZE_PTR(op_array->live_range);
@@ -1394,6 +1408,14 @@ static void zend_file_cache_unserialize_op_array(zend_op_array           *op_arr
 			}
 		}
 
+		if (op_array->num_dynamic_func_defs) {
+			UNSERIALIZE_PTR(op_array->dynamic_func_defs);
+			for (uint32_t i = 0; i < op_array->num_dynamic_func_defs; i++) {
+				UNSERIALIZE_PTR(op_array->dynamic_func_defs[i]);
+				zend_file_cache_unserialize_op_array(op_array->dynamic_func_defs[i], script, buf);
+			}
+		}
+
 		UNSERIALIZE_STR(op_array->function_name);
 		UNSERIALIZE_STR(op_array->filename);
 		UNSERIALIZE_PTR(op_array->live_range);
diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c
index 5e406ec4b6..6688e473cb 100644
--- a/ext/opcache/zend_persist.c
+++ b/ext/opcache/zend_persist.c
@@ -82,6 +82,7 @@
 typedef void (*zend_persist_func_t)(zval*);
 
 static void zend_persist_zval(zval *z);
+static void zend_persist_op_array(zval *zv);
 
 static const uint32_t uninitialized_bucket[-HT_MIN_MASK] =
 	{HT_INVALID_IDX, HT_INVALID_IDX};
@@ -663,6 +664,17 @@ static void zend_persist_op_array_ex(zend_op_array *op_array, zend_persistent_sc
 		}
 	}
 
+	if (op_array->num_dynamic_func_defs) {
+		op_array->dynamic_func_defs = zend_shared_memdup_put_free(
+			op_array->dynamic_func_defs, sizeof(zend_function *) * op_array->num_dynamic_func_defs);
+		for (uint32_t i = 0; i < op_array->num_dynamic_func_defs; i++) {
+			zval tmp;
+			ZVAL_PTR(&tmp, op_array->dynamic_func_defs[i]);
+			zend_persist_op_array(&tmp);
+			op_array->dynamic_func_defs[i] = Z_PTR(tmp);
+		}
+	}
+
 	ZCG(mem) = (void*)((char*)ZCG(mem) + ZEND_ALIGNED_SIZE(zend_extensions_op_array_persist(op_array, ZCG(mem))));
 
 #ifdef HAVE_JIT
@@ -676,16 +688,22 @@ static void zend_persist_op_array_ex(zend_op_array *op_array, zend_persistent_sc
 static void zend_persist_op_array(zval *zv)
 {
 	zend_op_array *op_array = Z_PTR_P(zv);
-
+	zend_op_array *old_op_array;
 	ZEND_ASSERT(op_array->type == ZEND_USER_FUNCTION);
-	op_array = Z_PTR_P(zv) = zend_shared_memdup(Z_PTR_P(zv), sizeof(zend_op_array));
-	zend_persist_op_array_ex(op_array, NULL);
-	if (!ZCG(current_persistent_script)->corrupted) {
-		op_array->fn_flags |= ZEND_ACC_IMMUTABLE;
-		ZEND_MAP_PTR_NEW(op_array->run_time_cache);
-		if (op_array->static_variables) {
-			ZEND_MAP_PTR_NEW(op_array->static_variables_ptr);
+
+	old_op_array = zend_shared_alloc_get_xlat_entry(op_array);
+	if (!old_op_array) {
+		op_array = Z_PTR_P(zv) = zend_shared_memdup_put(Z_PTR_P(zv), sizeof(zend_op_array));
+		zend_persist_op_array_ex(op_array, NULL);
+		if (!ZCG(current_persistent_script)->corrupted) {
+			op_array->fn_flags |= ZEND_ACC_IMMUTABLE;
+			ZEND_MAP_PTR_NEW(op_array->run_time_cache);
+			if (op_array->static_variables) {
+				ZEND_MAP_PTR_NEW(op_array->static_variables_ptr);
+			}
 		}
+	} else {
+		/* This can happen during preloading, if a dynamic function definition is declared. */
 	}
 }
 
diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c
index 015490b07a..ad27d539be 100644
--- a/ext/opcache/zend_persist_calc.c
+++ b/ext/opcache/zend_persist_calc.c
@@ -46,6 +46,7 @@
 	} while (0)
 
 static void zend_persist_zval_calc(zval *z);
+static void zend_persist_op_array_calc(zval *zv);
 
 static void zend_hash_persist_calc(HashTable *ht)
 {
@@ -287,16 +288,29 @@ static void zend_persist_op_array_calc_ex(zend_op_array *op_array)
 		}
 	}
 
+	if (op_array->num_dynamic_func_defs) {
+		ADD_SIZE(sizeof(void *) * op_array->num_dynamic_func_defs);
+		for (uint32_t i = 0; i < op_array->num_dynamic_func_defs; i++) {
+			zval tmp;
+			ZVAL_PTR(&tmp, op_array->dynamic_func_defs[i]);
+			zend_persist_op_array_calc(&tmp);
+		}
+	}
+
 	ADD_SIZE(ZEND_ALIGNED_SIZE(zend_extensions_op_array_persist_calc(op_array)));
 }
 
 static void zend_persist_op_array_calc(zval *zv)
 {
 	zend_op_array *op_array = Z_PTR_P(zv);
-
 	ZEND_ASSERT(op_array->type == ZEND_USER_FUNCTION);
-	ADD_SIZE(sizeof(zend_op_array));
-	zend_persist_op_array_calc_ex(Z_PTR_P(zv));
+	if (!zend_shared_alloc_get_xlat_entry(op_array)) {
+		zend_shared_alloc_register_xlat_entry(op_array, op_array);
+		ADD_SIZE(sizeof(zend_op_array));
+		zend_persist_op_array_calc_ex(op_array);
+	} else {
+		/* This can happen during preloading, if a dynamic function definition is declared. */
+	}
 }
 
 static void zend_persist_class_method_calc(zval *zv)