Bug #81104 [Opn->Csd]: Warning: "Failed to set memory limit to ... bytes" emitted after exit in debug

Edit report at https://bugs.php.net/bug.php?id=81104&edit=1 ID: 81104 Updated by: git@php.net Reported by: tandre@php.net Summary: Warning: "Failed to set memory limit to ... bytes" emitted after exit in debug -Status: Open +Status: Closed Type: Bug Package: Scripting Engine problem PHP Version: master-Git-2021-06-03 (Git) Block user comment: N Private report: N New Comment: Automatic comment on behalf of nikic Revision: https://github.com/php/php-src/commit/d8165c2502516dfda506e2f27aadec58041273ff Log: Fixed bug #81104 Previous Comments: ------------------------------------------------------------------------ [2021-06-04 13:25:44] pvandommelen at gmail dot com > Can't we just skip the warning during shutdown? The warning indicates that the memory reset failed. Wouldn't that be an issue for something like fpm? I'm not familiar with how the request isolation works though. ------------------------------------------------------------------------ [2021-06-04 13:06:32] nikic@php.net I don't think it's sufficient to suppress the warning during STAGE_DEACTIVATE, as the actual setting of the memory limit will fail as well, and we do want that to happen. We should probably combine suppressing the warning with an explicit setting of the memory limit after shutdown_memory_manager(), where it should definitely be safe to set it (unless it was invalid in the first place). This change is also in PHP 7.4/8.0. I think we should revert it there without replacement. Changes to memory limit handling in stable versions are too risky. ------------------------------------------------------------------------ [2021-06-04 11:40:42] cmb@php.net Can't we just skip the warning during shutdown? ------------------------------------------------------------------------ [2021-06-04 08:24:44] pvandommelen at gmail dot com I expect the commit from https://github.com/php/php-src/commit/1b3b5c94e52d10eb7a3f69b486a51b3f4d214d4f to have caused this. Silently accepting the invalid memory limit during request shutdown is probably not a good idea. We could try to always run garbage collection during request shutdown. Any errors that would still occur after that would be legitimate memory leaks. But that's probably an unacceptable performance loss for some command line scripts where running garbage collection is absolutely unnecessary. Preferred solution would probably be to fix the issue where an out-of-memory error can be thrown without garbage collection being called at all (https://bugs.php.net/bug.php?id=60982), and also applying that logic to this new error. I don't know how hard that would be to implement. Alternatively the commit could be reverted, maybe in favour of the earlier variant which resolved the integer underflow using runtime checks. ------------------------------------------------------------------------ [2021-06-03 23:43:12] tandre@php.net I suspect that https://github.com/php/php-src/commit/e9b005158f689be47f318a544b236fd5c64a9ab3#diff-2978fe1c2c45b4eca89dc476376ddc7193bc4e5e7fff0c7d1c465f057b35a5e6L1791 may be related. It has a check for (size_t)PG(memory_limit) < zend_memory_usage(1) that was removed - I assume it was discarding all errors if the current memory usage exceeds the original system memory limit (but am not deeply familiar with the output control code) ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=81104 -- Edit this bug report at https://bugs.php.net/bug.php?id=81104&edit=1