-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Addition of a max memory limit ini setting #17951
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.
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
Comments
The alternative might be making |
@NattyNarwhal that would be a great alternative and probably a lot easier to implement. The whole idea of changing the |
It's unusual to me as well; I suspect it might have been locked down with safe mode in the before times, but that's since been long gone. Making it per-dir/system is trivial, but it might be a breaking change and/or justify an RFC. I'll let others chime in on that (and the history too). |
Changing config at runtime is useful, I tend to keep all config at default and modify it at runtime as I see fit, it save me the trouble of changing config per script.. |
@kocoten1992 My concern specifically with |
Yes, more lock down would be better in enterprise or hosting service environment. In your case, would add |
@kocoten1992 Adding That’s why I’m suggesting a I see your concern that PHP already got a lot of config options. Perhaps you'd be more inclined to the alternative solution of making |
I understand your problem better now, change config at runtime is fine, except when it come to resource allocation? It could be a new class of problem. You have better chance discuss on php mailing list and propose a solution for your problem (web interface at https://externals.io/). |
It's a valid strategy to keep memory limit as low as possible, and increase it for endpoints that need it (e.g. the ones opening large files). If those endpoints are hidden behind authentication, this can potentially protect you against certain DoS attacks. Locking down Generally, it's your own responsibility to set these values appropriately, whether globally or at runtime. If some vendor code sets it unreasonably high, IMO, this should be fixed on their part. |
Agree we should not lock memory_limit. If anything, it should be a new INI. |
@iluuu1994 that's why I initially suggested a new INI |
Sure, I wouldn't object to an upper limit. |
@iluuu1994 It seems there's more support for the |
@frederikmillingpytlick This doesn't necessarily need an RFC. If you want, you can create a patch and then send an email to the internals mailing list. If you're not interested in implementing it yourself, you can ask for help in your email to the list. If somebody on the list were to object, you can create a short RFC. |
@iluuu1994 I'm in the middle of fiddling with a patch for it and came up with a thought. What do you think about adjusting the value to the max allowed in case it's above the allowed value? Code example: memory_limit = 128M
max_memory_limit = 256M
<?php
echo ini_get('memory_limit'); // Output: 128M
ini_set('memory_limit', '257M'); // PHP Warning: memory_limit (currently: 134217728 bytes) cannot exceed max_memory_limit (268435456 bytes). Attempting to raise memory_limit up to max_memory_limit.
// Set's memory_limit to max allowed value (256M)
echo ini_get('memory_limit'); // Output: 256M |
Setting the value above what is allowed should error and not change the value. We actually already do that when you're trying to set Lines 325 to 334 in a14301b
Importantly, |
Description
I'd like a
max_memory_limit
(naming?) ini setting to ensure that PHP never raises thememory_limit
ini setting higher than a certain point. This setting should not be editable usingini_set
. I'd be open to placing themax_memory_limit
setting somewhere else.Example scenario:
Running a PHP application in Kubernetes you'd have set some resource constraints. Let's say you've allocated 256 MB to a pod running the application and set
memory_limit
to 128 MB inphp.ini
.This setup means we'll have 128 MB available in PHP and a buffer of 128 MB for the rest of the pod. (Assuming only one PHP script will be executed simultaneously, since
memory_limit
is per script)Now to the issue. PHP has support for changing the
memory_limit
setting using theini_set
function during runtime.If the
memory_limit
is set to a value higher than the total allocated memory in the pod (256 MB in this scenario), then when the pod's memory of 256 MB has been exhausted by PHP, it will be the kernel OOM killer that handles the exhaustion instead of PHP. The OOM killer then sends PHP a SIGKILL signal which causes a non-graceful exit. (Meaning that potential shutdown handler(s) registered usingregister_shutdown_function()
won't get to run)Suggested behaviour
php.ini
:PHP code
:The text was updated successfully, but these errors were encountered: