8000 Fix GH-17951: Addition of max_memory_limit INI by frederikpyt · Pull Request #18011 · php/php-src · GitHub
[go: up one dir, main page]
Skip to content

Fix GH-17951: Addition of max_memory_limit INI #18011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+209 −2
Open

Fix GH-17951: Addition of max_memory_limit INI #18011

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9341c6e
Fix GH-17951: Addition of max_memory_limit INI
frederikpyt Mar 10, 2025
25c4028
Fix format specifiers to match type & utilize stage
frederikpyt Mar 10, 2025
2d61446
Fix format specifiers to be platform specific
frederikpyt Mar 10, 2025
9e42b57
Fix scenario where max_memory_limit could be lower than memory_limit
frederikpyt Mar 10, 2025
da70576
Fix issues addressed during review
frederikpyt Mar 10, 2025
72f4df4
Made it so max_memory_limit INI is set before memory_limit INI
8000 frederikpyt Mar 11, 2025
ffe7913
Fix whitespace and remove vim comments
frederikpyt Mar 11, 2025
1d49976
Changed fatals to warnings during startup
frederikpyt Apr 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Made it so max_memory_limit INI is set before memory_limit INI
  • Loading branch information
@frederikpyt
frederikpyt committed Mar 11, 2025
commit 72f4df495a978191d166d3e820fa5b3ff1ccd135
33 changes: 9 additions & 24 deletions main/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,10 @@ static PHP_INI_MH(OnChangeMemoryLimit)
}

/* If max_memory_limit is not set to unlimited, verify change */
if (PG(max_memory_limit) != 0 && PG(max_memory_limit) != -1) {
if (PG(max_memory_limit) != -1) {
if (value == -1) {
zend_error(
E_WARNING,
stage == ZEND_INI_STAGE_STARTUP ? E_ERROR : E_WARNING,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again, I think it would be better to stick to a warning in all cases. Erroring is very harsh, especially on startup, taking down the entire website.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change all cases to a warning :)

"Failed to set memory_limit to unlimited. memory_limit (currently: " ZEND_LONG_FMT " bytes) cannot be set to unlimited if max_memory_limit (" ZEND_LONG_FMT " bytes) is not unlimited",
PG(memory_limit),
PG(max_memory_limit)
Expand All @@ -339,7 +339,7 @@ static PHP_INI_MH(OnChangeMemoryLimit)

if (value > PG(max_memory_limit)) {
zend_error(
E_WARNING,
stage == ZEND_INI_STAGE_STARTUP ? E_ERROR : E_WARNING,
"Failed to set memory_limit to %zd bytes. memory_limit (currently: " ZEND_LONG_FMT " bytes) cannot exceed max_memory_limit (" ZEND_LONG_FMT " bytes)",
value,
PG(memory_limit),
Expand Down Expand Up @@ -376,29 +376,14 @@ static PHP_INI_MH(OnChangeMaxMemoryLimit)
value = Z_L(1) << 30; /* effectively, no limit */
}

/* If new value is not unlimited, verify change */
if (value != -1) {
if (PG(memory_limit) == -1) {
zend_error(
E_ERROR,
"memory_limit is set to unlimited, you cannot set max_memory_limit to %zd bytes",
value
);
return FAILURE;
}

if (PG(memory_limit) > value) {
zend_error(
E_ERROR,
"memory_limit (" ZEND_LONG_FMT " bytes) exceeds max_memory_limit (%zd bytes)",
PG(memory_limit),
value
);
return FAILURE;
}
if (zend_set_memory_limit(value) == FAILURE) {
zend_error(E_ERROR, "Failed to set memory limit to %zd bytes (Current memory usage is %zd bytes)", value, zend_memory_usage(true));
return FAILURE;
}

PG(memory_limit) = value;
PG(max_memory_limit) = value;

return SUCCESS;
}
/* }}} */
Expand Down Expand Up @@ -867,8 +852,8 @@ PHP_INI_BEGIN()
STD_PHP_INI_ENTRY("mail.log", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateMailLog, mail_log, php_core_globals, core_globals)
PHP_INI_ENTRY("browscap", NULL, PHP_INI_SYSTEM, OnChangeBrowscap)

PHP_INI_ENTRY("memory_limit", "128M", PHP_INI_ALL, OnChangeMemoryLimit)
PHP_INI_ENTRY("max_memory_limit", "-1", PHP_INI_SYSTEM, OnChangeMaxMemoryLimit)
PHP_INI_ENTRY("memory_limit", "128M", PHP_INI_ALL, OnChangeMemoryLimit)

PHP_INI_ENTRY("precision", "14", PHP_INI_ALL, OnSetPrecision)
PHP_INI_ENTRY("sendmail_from", NULL, PHP_INI_ALL, NULL)
Expand Down
6 changes: 4 additions & 2 deletions tests/basic/gh17951_ini_parse_1.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ memory_limit=128M
max_memory_limit=-1
--FILE--
<?php
echo "OK";
echo ini_get('max_memory_limit') . PHP_EOL;
echo ini_get('memory_limit') . PHP_EOL;
--EXPECT--
OK
-1
128M
6 changes: 4 additions & 2 deletions tests/basic/gh17951_ini_parse_2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ memory_limit=-1
max_memory_limit=-1
--FILE--
<?php
echo "OK";
echo ini_get('max_memory_limit') . PHP_EOL;
echo ini_get('memory_limit') . PHP_EOL;
--EXPECT--
OK
-1
-1
6 changes: 4 additions & 2 deletions tests/basic/gh17951_ini_parse_3.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ memory_limit=128M
max_memory_limit=256M
--FILE--
<?php
echo "OK";
echo ini_get('max_memory_limit') . PHP_EOL;
echo ini_get('memory_limit') . PHP_EOL;
--EXPECT--
OK
256M
128M
6 changes: 3 additions & 3 deletions tests/basic/gh17951_ini_parse_4.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ max_memory_limit=128M
echo ini_get('max_memory_limit') . PHP_EOL;
echo ini_get('memory_limit') . PHP_EOL;
--EXPECTF--
Fatal error: memory_limit is set to unlimited, you cannot set max_memory_limit to %d bytes in %s
-1
-1
Fatal error: Failed to set memory_limit to unlimited. memory_limit (currently: %d bytes) cannot be set to unlimited if max_memory_limit (%d bytes) is not unlimited in %s
128M
128M
6 changes: 3 additions & 3 deletions tests/basic/gh17951_ini_parse_5.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ max_memory_limit=128M
echo ini_get('max_memory_limit') . PHP_EOL;
echo ini_get('memory_limit') . PHP_EOL;
--EXPECTF--
Fatal error: memory_limit (%d bytes) exceeds max_memory_limit (%d bytes) in %s
-1
256M
Fatal error: Failed to set memory_limit to %d bytes. memory_limit (currently: %d bytes) cannot exceed max_memory_limit (%d bytes) in %s
128M
128M
Loading
0