Skip to content

Commit 7682868

Browse files
nielsdosnielsdos
committed
Merge branch 'PHP-8.1' into PHP-8.2
* PHP-8.1: Fix GH-8065: opcache.consistency_checks > 0 causes segfaults in PHP >= 8.1.5 in fpm context Fix GH-8646: Memory leak PHP FPM 8.1
2 parents 805dafd + 85df512 commit 7682868

File tree

8 files changed

+138
-4
lines changed

8 files changed

+138
-4
lines changed

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@ 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)
1213

1314
- Opcache:
1415
. Fixed build for macOS to cater with pkg-config settings. (David Carlier)
16+
. Fixed bug GH-8065 (opcache.consistency_checks > 0 causes segfaults in
17+
PHP >= 8.1.5 in fpm context). (nielsdos)
1518

1619
- OpenSSL:
1720
. Add missing error checks on file writing functions. (nielsdos)

‎Zend/zend.c‎

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

12681268
zend_destroy_rsrc_list(&EG(regular_list));
12691269

1270+
/* See GH-8646: https://github.com/php/php-src/issues/8646
1271+
*
1272+
* Interned strings that hold class entries can get a corresponding slot in map_ptr for the CE cache.
1273+
* map_ptr works like a bump allocator: there is a counter which increases to allocate the next slot in the map.
1274+
*
1275+
* For class name strings in non-opcache we have:
1276+
* - on startup: permanent + interned
1277+
* - on request: interned
1278+
* For class name strings in opcache we have:
1279+
* - on startup: permanent + interned
1280+
* - on request: either not interned at all, which we can ignore because they won't get a CE cache entry
1281+
* or they were already permanent + interned
1282+
* or we get a new permanent + interned string in the opcache persistence code
1283+
*
1284+
* Notice that the map_ptr layout always has the permanent strings first, and the request strings after.
1285+
* In non-opcache, a request string may get a slot in map_ptr, and that interned request string
1286+
* gets destroyed at the end of the request. The corresponding map_ptr slot can thereafter never be used again.
1287+
* This causes map_ptr to keep reallocating to larger and larger sizes.
1288+
*
1289+
* We solve it as follows:
1290+
* We can check whether we had any interned request strings, which only happens in non-opcache.
1291+
* If we have any, we reset map_ptr to the last permanent string.
1292+
* We can't lose any permanent strings because of map_ptr's layout.
1293+
*/
1294+
if (zend_hash_num_elements(&CG(interned_strings)) > 0) {
1295+
zend_map_ptr_reset();
1296+
}
1297+
12701298
#if GC_BENCH
12711299
fprintf(stderr, "GC Statistics\n");
12721300
fprintf(stderr, "-------------\n");

‎ext/opcache/tests/gh8065.phpt‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-8065: opcache.consistency_checks > 0 causes segfaults in PHP >= 8.1.5 in fpm context
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.enable_cli=1
7+
opcache.consistency_checks=1
8+
opcache.log_verbosity_level=2
9+
--FILE--
10+
<?php
11+
var_dump(ini_get("opcache.consistency_checks"));
12+
var_dump(ini_set("opcache.consistency_checks", 1));
13+
var_dump(ini_set("opcache.consistency_checks", -1));
14+
var_dump(ini_set("opcache.consistency_checks", 0));
15+
?>
16+
--EXPECTF--
17+
%sWarning opcache.consistency_checks is reset back to 0 because it does not work properly (see GH-8065, GH-10624).
18+
19+
string(1) "0"
20+
%sWarning opcache.consistency_checks is reset back to 0 because it does not work properly (see GH-8065, GH-10624).
21+
22+
bool(false)
23+
%sWarning opcache.consistency_checks is reset back to 0 because it does not work properly (see GH-8065, GH-10624).
24+
25+
bool(false)
26+
string(1) "0"

‎ext/opcache/zend_accelerator_module.c‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,19 @@ static ZEND_INI_MH(OnUpdateMaxWastedPercentage)
128128
return SUCCESS;
129129
}
130130

131+
static ZEND_INI_MH(OnUpdateConsistencyChecks)
132+
{
133+
zend_long *p = (zend_long *) ZEND_INI_GET_ADDR();
134+
zend_long consistency_checks = atoi(ZSTR_VAL(new_value));
135+
136+
if (consistency_checks != 0) {
137+
zend_accel_error(ACCEL_LOG_WARNING, "opcache.consistency_checks is reset back to 0 because it does not work properly (see GH-8065, GH-10624).\n");
138+
return FAILURE;
139+
}
140+
*p = 0;
141+
return SUCCESS;
142+
}
143+
131144
static ZEND_INI_MH(OnEnable)
132145
{
133146
if (stage == ZEND_INI_STAGE_STARTUP ||
@@ -262,7 +275,7 @@ ZEND_INI_BEGIN()
262275
STD_PHP_INI_ENTRY("opcache.interned_strings_buffer", "8" , PHP_INI_SYSTEM, OnUpdateInternedStringsBuffer, accel_directives.interned_strings_buffer, zend_accel_globals, accel_globals)
263276
STD_PHP_INI_ENTRY("opcache.max_accelerated_files" , "10000", PHP_INI_SYSTEM, OnUpdateMaxAcceleratedFiles, accel_directives.max_accelerated_files, zend_accel_globals, accel_globals)
264277
STD_PHP_INI_ENTRY("opcache.max_wasted_percentage" , "5" , PHP_INI_SYSTEM, OnUpdateMaxWastedPercentage, accel_directives.max_wasted_percentage, zend_accel_globals, accel_globals)
265-
STD_PHP_INI_ENTRY("opcache.consistency_checks" , "0" , PHP_INI_ALL , OnUpdateLong, accel_directives.consistency_checks, zend_accel_globals, accel_globals)
278+
STD_PHP_INI_ENTRY("opcache.consistency_checks" , "0" , PHP_INI_ALL , OnUpdateConsistencyChecks, accel_directives.consistency_checks, zend_accel_globals, accel_globals)
266279
STD_PHP_INI_ENTRY("opcache.force_restart_timeout" , "180" , PHP_INI_SYSTEM, OnUpdateLong, accel_directives.force_restart_timeout, zend_accel_globals, accel_globals)
267280
STD_PHP_INI_ENTRY("opcache.revalidate_freq" , "2" , PHP_INI_ALL , OnUpdateLong, accel_directives.revalidate_freq, zend_accel_globals, accel_globals)
268281
STD_PHP_INI_ENTRY("opcache.file_update_protection", "2" , PHP_INI_ALL , OnUpdateLong, accel_directives.file_update_protection, zend_accel_globals, accel_globals)

‎ext/zend_test/test.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,12 @@ static ZEND_FUNCTION(zend_test_parameter_with_attribute)
457457
RETURN_LONG(1);
458458
}
459459

460+
static ZEND_FUNCTION(zend_get_map_ptr_last)
461+
{
462+
ZEND_PARSE_PARAMETERS_NONE();
463+
RETURN_LONG(CG(map_ptr_last));
464+
}
465+
460466
static zend_object *zend_test_class_new(zend_class_entry *class_type)
461467
{
462468
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
@@ -164,6 +164,8 @@ function zend_test_zend_ini_parse_quantity(string $str): int {}
164164
function zend_test_zend_ini_parse_uquantity(string $str): int {}
165165

166166
function zend_test_zend_ini_str(): string {}
167+
168+
function zend_get_map_ptr_last(): int {}
167169
}
168170

169171
namespace ZendTestNS {

‎ext/zend_test/test_arginfo.h‎

Lines changed: 7 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎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)