Repository navigation
Memory leak when class and trait declare same static property with doc block in 8.3 #12207
Description
Description
When building the current PHP-8.3 branch (tested with commit 99cd81c) in debug mode, PHP reports a memory leak when executing the following code:
<?php
trait MyTrait {
/**
* trait comment
*/
static $property;
}
class MyClass {
use MyTrait;
/**
* class comment
*/
static $property;
}Resulted in this output:
$ sapi/cli/php test.php
[Wed Sep 13 23:02:06 2023] Script: '/Users/florian/projects/php/php-src/test.php'
Zend/zend_string.h(174) : Freeing 0x0000000107c4d540 (64 bytes), script=/Users/florian/projects/php/php-src/test.php
=== Total 1 memory leaks detected ===
The doc block from the class is leaked. I had the same behavior with the 8.3 beta 3. The issue did not occur in 8.2 for me.
The following change fixes the issue for me:
diff --git a/Zend/zend_API.c b/Zend/zend_API.c
index d629b7166a..22ed008004 100644
--- a/Zend/zend_API.c
+++ b/Zend/zend_API.c
@@ -4332,6 +4332,9 @@ ZEND_API zend_property_info *zend_declare_typed_property(zend_class_entry *ce, z
(property_info_ptr->flags & ZEND_ACC_STATIC) != 0) {
property_info->offset = property_info_ptr->offset;
zval_ptr_dtor(&ce->default_static_members_table[property_info->offset]);
+ if (property_info_ptr->doc_comment) {
+ zend_string_release_ex(property_info_ptr->doc_comment, 0);
+ }
zend_hash_del(&ce->properties_info, name);
} else {
property_info->offset = ce->default_static_members_count++;But I don't think thats a proper fix for the issue.
PHP Version
PHP 8.3
Operating System
Mac OS X
Activity
devnexen commentedon Sep 15, 2023
Not sure it's wrong but it might need more completion perhaps but I will defer to @Girgias @nielsdos .
ndossche commentedon Sep 16, 2023
I think the suggested fix is almost right, except that I believe doc_comment may also be persistent. So please use zend_string_release instead of zend_string_release_ex.
Please create a pull request :)
devnexen commentedon Sep 16, 2023
quick question, would we need to do the same here ?
ndossche commentedon Sep 16, 2023
That code is only for internal classes. I don't know if we actively support doc comments for internal classes (e.g. via stub generation). Probably, for completeness we should do the same fix there too as it's possible to call the API manually anyway. For that case, you may use
zend_string_release_ex(property_info->doc_comment, 1);as internal classes are always persistent.rioderelfte commentedon Sep 18, 2023
Thanks for the input!
I found this
zend_string_release_exhere:php-src/Zend/zend_opcode.c
Line 388 in 3e0e7e3
Thats why I used it in my patch as well. But I will create a PR with
zend_string_releaseand also will add the release to theelse.Fix phpGH-12207 memory leak of doc blocks of static properties