Fix GH-8646: Memory leak PHP FPM 8.1

Fixes GH-8646
See #8646 for thorough discussion.

Interned strings that hold class entries can get a corresponding slot in map_ptr for the CE cache.
map_ptr works like a bump allocator: there is a counter which increases to allocate the next slot in the map.

For class name strings in non-opcache we have:
  - on startup: permanent + interned
  - on request: interned
For class name strings in opcache we have:
  - on startup: permanent + interned
  - on request: either not interned at all, which we can ignore because they won't get a CE cache entry
                or they were already permanent + interned
                or we get a new permanent + interned string in the opcache persistence code

Notice that the map_ptr layout always has the permanent strings first, and the request strings after.
In non-opcache, a request string may get a slot in map_ptr, and that interned request string
gets destroyed at the end of the request. The corresponding map_ptr slot can thereafter never be used again.
This causes map_ptr to keep reallocating to larger and larger sizes.

We solve it as follows:
We can check whether we had any interned request strings, which only happens in non-opcache.
If we have any, we reset map_ptr to the last permanent string.
We can't lose any permanent strings because of map_ptr's layout.

Author: nielsdos

ext/zend_test/test.c

@@ -321,6 +321,12 @@ static ZEND_FUNCTION(zend_test_parameter_with_attribute)
 	RETURN_LONG(1);
 }
 
+static ZEND_FUNCTION(zend_get_map_ptr_last)
+{
+	ZEND_PARSE_PARAMETERS_NONE();
+	RETURN_LONG(CG(map_ptr_last));
+}
+
 static zend_object *zend_test_class_new(zend_class_entry *class_type)
 {
 	zend_object *obj = zend_objects_new(class_type);

ext/zend_test/test.stub.php

@@ -115,6 +115,8 @@ function zend_test_parameter_with_attribute(string $parameter): int {}
     function zend_get_current_func_name(): string {}
 
     function zend_call_method(string $class, string $method, mixed $arg1 = UNKNOWN, mixed $arg2 = UNKNOWN): mixed {}
+
+    function zend_get_map_ptr_last(): int {}
 }
 
 namespace ZendTestNS {

ext/zend_test/test_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 27df6a7b48574b5c6c9a54c618fce300c7a8bd13 */
+ * Stub hash: 1eca5b01969498e67501a59dc69ba4c01263c4d9 */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0)
 ZEND_END_ARG_INFO()
@@ -79,12 +79,14 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_call_method, 0, 2, IS_MIXED
 	ZEND_ARG_TYPE_INFO(0, arg2, IS_MIXED, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_ZendSubNS_namespaced_func, 0, 0, _IS_BOOL, 0)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_get_map_ptr_last, 0, 0, IS_LONG, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class__ZendTestClass_is_object, 0, 0, IS_LONG, 0)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_ZendSubNS_namespaced_func, 0, 0, _IS_BOOL, 0)
 ZEND_END_ARG_INFO()
 
+#define arginfo_class__ZendTestClass_is_object arginfo_zend_get_map_ptr_last
+
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class__ZendTestClass___toString, 0, 0, 0)
 ZEND_END_ARG_INFO()
 
@@ -136,6 +138,7 @@ static ZEND_FUNCTION(zend_get_unit_enum);
 static ZEND_FUNCTION(zend_test_parameter_with_attribute);
 static ZEND_FUNCTION(zend_get_current_func_name);
 static ZEND_FUNCTION(zend_call_method);
+static ZEND_FUNCTION(zend_get_map_ptr_last);
 static ZEND_FUNCTION(namespaced_func);
 static ZEND_METHOD(_ZendTestClass, is_object);
 static ZEND_METHOD(_ZendTestClass, __toString);
@@ -173,6 +176,7 @@ static const zend_function_entry ext_functions[] = {
 	ZEND_FE(zend_test_parameter_with_attribute, arginfo_zend_test_parameter_with_attribute)
 	ZEND_FE(zend_get_current_func_name, arginfo_zend_get_current_func_name)
 	ZEND_FE(zend_call_method, arginfo_zend_call_method)
+	ZEND_FE(zend_get_map_ptr_last, arginfo_zend_get_map_ptr_last)
 	ZEND_NS_FE("ZendTestNS2\\ZendSubNS", namespaced_func, arginfo_ZendTestNS2_ZendSubNS_namespaced_func)
 	ZEND_FE_END
 };

main/main.c

@@ -1779,6 +1779,37 @@ int php_request_startup(void)
 }
 /* }}} */
 
+static void map_ptr_reset_request_strings(void)
+{
+	/* See GH-8646: https://github.com/php/php-src/issues/8646
+	 *
+	 * Interned strings that hold class entries can get a corresponding slot in map_ptr for the CE cache.
+	 * map_ptr works like a bump allocator: there is a counter which increases to allocate the next slot in the map.
+	 *
+	 * For class name strings in non-opcache we have:
+	 *   - on startup: permanent + interned
+	 *   - on request: interned
+	 * For class name strings in opcache we have:
+	 *   - on startup: permanent + interned
+	 *   - on request: either not interned at all, which we can ignore because they won't get a CE cache entry
+	 *                 or they were already permanent + interned
+	 *                 or we get a new permanent + interned string in the opcache persistence code
+	 *
+	 * Notice that the map_ptr layout always has the permanent strings first, and the request strings after.
+	 * In non-opcache, a request string may get a slot in map_ptr, and that interned request string
+	 * gets destroyed at the end of the request. The corresponding map_ptr slot can thereafter never be used again.
+	 * This causes map_ptr to keep reallocating to larger and larger sizes.
+	 *
+	 * We solve it as follows:
+	 * We can check whether we had any interned request strings, which only happens in non-opcache.
+	 * If we have any, we reset map_ptr to the last permanent string.
+	 * We can't lose any permanent strings because of map_ptr's layout.
+	 */
+	if (zend_hash_num_elements(&CG(interned_strings)) > 0) {
+		zend_map_ptr_reset();
+	}
+}
+
 /* {{{ php_request_shutdown */
 void php_request_shutdown(void *dummy)
 {
@@ -1872,6 +1903,7 @@ void php_request_shutdown(void *dummy)
 
 	/* 15. Free Willy (here be crashes) */
 	zend_arena_destroy(CG(arena));
+	map_ptr_reset_request_strings();
 	zend_interned_strings_deactivate();
 	zend_try {
 		shutdown_memory_manager(CG(unclean_shutdown) || !report_memleaks, 0);

sapi/fpm/tests/gh8646.phpt

@@ -0,0 +1,52 @@
+--TEST--
+GH-8646 (Memory leak PHP FPM 8.1)
+--EXTENSIONS--
+zend_test
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+EOT;
+
+$code = <<<EOT
+<?php
+class MyClass {}
+echo zend_get_map_ptr_last();
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+$map_ptr_last_values = [];
+for ($i = 0; $i < 10; $i++) {
+    $map_ptr_last_values[] = (int) $tester->request()->getBody();
+}
+// Ensure that map_ptr_last did not increase
+var_dump(count(array_unique($map_ptr_last_values, SORT_REGULAR)) === 1);
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->close();
+
+?>
+Done
+--EXPECT--
+bool(true)
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>