php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #81104 Warning: "Failed to set memory limit to ... bytes" emitted after exit in debug
Submitted: 2021-06-03 23:38 UTC Modified: 2021-06-04 13:06 UTC
From: tandre@php.net Assigned:
Status: Closed Package: Scripting Engine problem
PHP Version: master-Git-2021-06-03 (Git) OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: tandre@php.net
New email:
PHP Version: OS:

 

 [2021-06-03 23:38 UTC] tandre@php.net
Description:
------------
This only happens when PHP is built with `--enable-debug`, and it's only a notice. Possibly, it's deliberate.

`php -d memory_limit=5M test.php` on the attached script will run the script successfully, and while php is shutting down, php will internally reset original ini settings, including memory_limit. This will trigger an error because the restored memory_limit is smaller than the amount of memory recorded as being in use at the time - this is more likely to affect programs with data structures that can't be garbage collected
(gc_disable(); public function __construct() { $this->x = [$this]; })

```
(gdb) b zend_error
Breakpoint 1 at 0x613f69: file /path/to/php-src/Zend/zend.c, line 1561.
(gdb) run
Starting program: /path/to/php-8.1.0-debug-intersection-types-install/bin/php -d memory_limit=5M test.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Memory usage before shutting down: 11975904

Breakpoint 1, zend_error (type=21845, format=0x555556379140 "\001") at /path/to/php-src/Zend/zend.c:1561
1561    ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) {
(gdb) bt
#0  zend_error (type=21845, format=0x555556379140 "\001") at /path/to/php-src/Zend/zend.c:1561
#1  0x0000555555ac1824 in OnChangeMemoryLimit (entry=0x55555637ebe0, new_value=0x55555637ec60, mh_arg1=0x0, mh_arg2=0x0, mh_arg3=0x0, stage=8) at /path/to/php-src/main/main.c:276
#2  0x0000555555c1d8eb in zend_restore_ini_entry_cb (ini_entry=0x55555637ebe0, stage=8) at /path/to/php-src/Zend/zend_ini.c:54
#3  0x0000555555c1db8c in zend_ini_deactivate () at /path/to/php-src/Zend/zend_ini.c:129
#4  0x0000555555b672ac in zend_deactivate () at /path/to/php-src/Zend/zend.c:1290
#5  0x0000555555ac4d17 in php_request_shutdown (dummy=0x0) at /path/to/php-src/main/main.c:1823
#6  0x0000555555cd21d2 in do_cli (argc=4, argv=0x555556378cb0) at /path/to/php-src/sapi/cli/php_cli.c:1134
#7  0x0000555555cd2ab6 in main (argc=4, argv=0x555556378cb0) at /path/to/php-src/sapi/cli/php_cli.c:1366
```

Perhaps the memory_limit should be a special case, and only be restored after everything else shuts down, and memory is freed?


This is emitted as a notice to stdout when running https://github.com/phan/phan to self-analyze itself in php 8.1 with the default memory limit of 138 MB (phan generates cyclic data structures in some places, such as union type representations)

Test script:
---------------
<?php
// Run this with `php -d memory_limit=5M test.php`
class X {
    public $x;
    public function __construct() { $this->x = [$this]; }
}
gc_disable();
ini_set('memory_limit', '1G');
$y = [];
for ($i = 0; $i < 20000; $i++) {
    $y[] = new X();
}
$y[0]->y = &$y;
$y[0]->z = $x;
echo "Memory usage before shutting down: " . memory_get_usage() . "\n";

// Observed in php 8.1:
// Warning: Failed to set memory limit to 5242880 bytes (Current memory usage is 12582912 bytes) in Unknown on line 0


Expected result:
----------------
No warning should be emitted during shutdown for a request

Actual result:
--------------
`Warning: Failed to set memory limit to 5242880 bytes (Current memory usage is 12582912 bytes) in Unknown on line 0` is emitted when php automatically tries to restore the system memory limit (`php -d memory_limit=5M`)


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-06-03 23:43 UTC] 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)
 [2021-06-04 08:24 UTC] 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-04 11:40 UTC] cmb@php.net
Can't we just skip the warning during shutdown?
 [2021-06-04 13:06 UTC] 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 13:25 UTC] 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-08 12:34 UTC] git@php.net
Automatic comment on behalf of nikic
Revision: https://github.com/php/php-src/commit/d8165c2502516dfda506e2f27aadec58041273ff
Log: Fixed bug #81104
 [2021-06-08 12:34 UTC] git@php.net
-Status: Open +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Apr 18 22:01:28 2024 UTC