Skip to content

Commit ff62d11

Browse files
committed
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. Closes GH-10783.
1 parent cfe1aab commit ff62d11

File tree

6 files changed

+96
-3
lines changed

6 files changed

+96
-3
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ PHP NEWS
66
. Added optional support for max_execution_time in ZTS/Linux builds
77
(Kévin Dunglas)
88
. Fixed use-after-free in recursive AST evaluation. (ilutov)
9+
. Fixed bug GH-8646 (Memory leak PHP FPM 8.1). (nielsdos)
910

1011
- FTP:
1112
. Propagate success status of ftp_close(). (nielsdos)

Zend/zend.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,6 +1286,34 @@ ZEND_API void zend_deactivate(void) /* {{{ */
12861286

12871287
zend_destroy_rsrc_list(&EG(regular_list));
12881288

1289+
/* See GH-8646: https://github.com/php/php-src/issues/8646
1290+
*
1291+
* Interned strings that hold class entries can get a corresponding slot in map_ptr for the CE cache.
1292+
* map_ptr works like a bump allocator: there is a counter which increases to allocate the next slot in the map.
1293+
*
1294+
* For class name strings in non-opcache we have:
1295+
* - on startup: permanent + interned
1296+
* - on request: interned
1297+
* For class name strings in opcache we have:
1298+
* - on startup: permanent + interned
1299+
* - on request: either not interned at all, which we can ignore because they won't get a CE cache entry
1300+
* or they were already permanent + interned
1301+
* or we get a new permanent + interned string in the opcache persistence code
1302+
*
1303+
* Notice that the map_ptr layout always has the permanent strings first, and the request strings after.
1304+
* In non-opcache, a request string may get a slot in map_ptr, and that interned request string
1305+
* gets destroyed at the end of the request. The corresponding map_ptr slot can thereafter never be used again.
1306+
* This causes map_ptr to keep reallocating to larger and larger sizes.
1307+
*
1308+
* We solve it as follows:
1309+
* We can check whether we had any interned request strings, which only happens in non-opcache.
1310+
* If we have any, we reset map_ptr to the last permanent string.
1311+
* We can't lose any permanent strings because of map_ptr's layout.
1312+
*/
1313+
if (zend_hash_num_elements(&CG(interned_strings)) > 0) {
1314+
zend_map_ptr_reset();
1315+
}
1316+
12891317
#if GC_BENCH
12901318
fprintf(stderr, "GC Statistics\n");
12911319
fprintf(stderr, "-------------\n");

ext/zend_test/test.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,12 @@ static ZEND_FUNCTION(zend_test_parameter_with_attribute)
321321
RETURN_LONG(1);
322322
}
323323

324+
static ZEND_FUNCTION(zend_get_map_ptr_last)
325+
{
326+
ZEND_PARSE_PARAMETERS_NONE();
327+
RETURN_LONG(CG(map_ptr_last));
328+
}
329+
324330
static zend_object *zend_test_class_new(zend_class_entry *class_type)
325331
{
326332
zend_object *obj = zend_objects_new(class_type);

ext/zend_test/test.stub.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ function zend_test_parameter_with_attribute(string $parameter): int {}
115115
function zend_get_current_func_name(): string {}
116116

117117
function zend_call_method(string $class, string $method, mixed $arg1 = UNKNOWN, mixed $arg2 = UNKNOWN): mixed {}
118+
119+
function zend_get_map_ptr_last(): int {}
118120
}
119121

120122
namespace ZendTestNS {

ext/zend_test/test_arginfo.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 27df6a7b48574b5c6c9a54c618fce300c7a8bd13 */
2+
* Stub hash: 1eca5b01969498e67501a59dc69ba4c01263c4d9 */
33

44
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0)
55
ZEND_END_ARG_INFO()
@@ -79,12 +79,14 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_call_method, 0, 2, IS_MIXED
7979
ZEND_ARG_TYPE_INFO(0, arg2, IS_MIXED, 0)
8080
ZEND_END_ARG_INFO()
8181

82-
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_ZendSubNS_namespaced_func, 0, 0, _IS_BOOL, 0)
82+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_get_map_ptr_last, 0, 0, IS_LONG, 0)
8383
ZEND_END_ARG_INFO()
8484

85-
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class__ZendTestClass_is_object, 0, 0, IS_LONG, 0)
85+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_ZendSubNS_namespaced_func, 0, 0, _IS_BOOL, 0)
8686
ZEND_END_ARG_INFO()
8787

88+
#define arginfo_class__ZendTestClass_is_object arginfo_zend_get_map_ptr_last
89+
8890
ZEND_BEGIN_ARG_INFO_EX(arginfo_class__ZendTestClass___toString, 0, 0, 0)
8991
ZEND_END_ARG_INFO()
9092

@@ -136,6 +138,7 @@ static ZEND_FUNCTION(zend_get_unit_enum);
136138
static ZEND_FUNCTION(zend_test_parameter_with_attribute);
137139
static ZEND_FUNCTION(zend_get_current_func_name);
138140
static ZEND_FUNCTION(zend_call_method);
141+
static ZEND_FUNCTION(zend_get_map_ptr_last);
139142
static ZEND_FUNCTION(namespaced_func);
140143
static ZEND_METHOD(_ZendTestClass, is_object);
141144
static ZEND_METHOD(_ZendTestClass, __toString);
@@ -173,6 +176,7 @@ static const zend_function_entry ext_functions[] = {
173176
ZEND_FE(zend_test_parameter_with_attribute, arginfo_zend_test_parameter_with_attribute)
174177
ZEND_FE(zend_get_current_func_name, arginfo_zend_get_current_func_name)
175178
ZEND_FE(zend_call_method, arginfo_zend_call_method)
179+
ZEND_FE(zend_get_map_ptr_last, arginfo_zend_get_map_ptr_last)
176180
ZEND_NS_FE("ZendTestNS2\\ZendSubNS", namespaced_func, arginfo_ZendTestNS2_ZendSubNS_namespaced_func)
177181
ZEND_FE_END
178182
};

sapi/fpm/tests/gh8646.phpt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
GH-8646 (Memory leak PHP FPM 8.1)
3+
--EXTENSIONS--
4+
zend_test
5+
--SKIPIF--
6+
<?php include "skipif.inc"; ?>
7+
--FILE--
8+
<?php
9+
10+
require_once "tester.inc";
11+
12+
$cfg = <<<EOT
13+
[global]
14+
error_log = {{FILE:LOG}}
15+
[unconfined]
16+
listen = {{ADDR}}
17+
pm = dynamic
18+
pm.max_children = 5
19+
pm.start_servers = 1
20+
pm.min_spare_servers = 1
21+
pm.max_spare_servers = 3
22+
EOT;
23+
24+
$code = <<<EOT
25+
<?php
26+
class MyClass {}
27+
echo zend_get_map_ptr_last();
28+
EOT;
29+
30+
$tester = new FPM\Tester($cfg, $code);
31+
$tester->start();
32+
$tester->expectLogStartNotices();
33+
$map_ptr_last_values = [];
34+
for ($i = 0; $i < 10; $i++) {
35+
$map_ptr_last_values[] = (int) $tester->request()->getBody();
36+
}
37+
// Ensure that map_ptr_last did not increase
38+
var_dump(count(array_unique($map_ptr_last_values, SORT_REGULAR)) === 1);
39+
$tester->terminate();
40+
$tester->expectLogTerminatingNotices();
41+
$tester->close();
42+
43+
?>
44+
Done
45+
--EXPECT--
46+
bool(true)
47+
Done
48+
--CLEAN--
49+
<?php
50+
require_once "tester.inc";
51+
FPM\Tester::clean();
52+
?>

0 commit comments

Comments
 (0)