Fix GH-8646: Memory leak PHP FPM 8.1

Fixes GH-8646
See https://github.com/php/php-src/issues/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.

Closes GH-10783.
This commit is contained in:
Niels Dossche 2023-03-04 23:22:05 +01:00
parent cfe1aab865
commit ff62d117a3
6 changed files with 96 additions and 3 deletions

1
NEWS
View File

@ -6,6 +6,7 @@ PHP NEWS
. Added optional support for max_execution_time in ZTS/Linux builds
(Kévin Dunglas)
. Fixed use-after-free in recursive AST evaluation. (ilutov)
. Fixed bug GH-8646 (Memory leak PHP FPM 8.1). (nielsdos)
- FTP:
. Propagate success status of ftp_close(). (nielsdos)

View File

@ -1286,6 +1286,34 @@ ZEND_API void zend_deactivate(void) /* {{{ */
zend_destroy_rsrc_list(&EG(regular_list));
/* 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();
}
#if GC_BENCH
fprintf(stderr, "GC Statistics\n");
fprintf(stderr, "-------------\n");

View File

@ -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);

View File

@ -115,6 +115,8 @@ namespace {
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 {

View File

@ -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,11 +79,13 @@ 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_zend_get_map_ptr_last, 0, 0, IS_LONG, 0)
ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_ZendSubNS_namespaced_func, 0, 0, _IS_BOOL, 0)
ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class__ZendTestClass_is_object, 0, 0, IS_LONG, 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
};

View File

@ -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();
?>