Bug #76874 [Ver]: xml_parser_free() should never leak memory

Edit report at https://bugs.php.net/bug.php?id=76874&edit=1 ID: 76874 Updated by: cmb@php.net Reported by: ksours at internetbrands dot com -Summary: Documentation of memory leak in Xml Parser +Summary: xml_parser_free() should never leak memory Status: Verified Type: Bug -Package: Documentation problem +Package: *XML functions -Operating System: Windows +Operating System: * PHP Version: 7.3.0beta3 -Assigned To: cmb +Assigned To: Block user comment: N Private report: N New Comment: The docs should be okay now, so changing to the actual bug. Previous Comments: ------------------------------------------------------------------------ [2018-09-14 12:09:45] cmb@php.net Automatic comment from SVN on behalf of cmb Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=345634 Log: Fix #76874: Documentation of memory leak in Xml Parser ------------------------------------------------------------------------ [2018-09-13 13:14:05] cmb@php.net Good catch, @ksours! This is not exactly the other bug, but it seems it has the same root cause. If an object references a resource, and this resource references that object, there is a cycle that could only be resolved by the cyclic GC, but since resources are not tracked (and that's unlikely to happen in the future), it won't. The proper solution would be to turn the parser resource into an object. ------------------------------------------------------------------------ [2018-09-12 20:42:51] ksours at internetbrands dot com Poked this a little farther and determined there is some more nuance than I originally understood. Here is the code to reproduce my case (can't seem to update the script field via the edit interface) <?php class c { private $xml; private $test; public function test() { $this->xml = xml_parser_create(); xml_set_character_data_handler($this->xml, array(&$this, 'handle_cdata')); xml_parser_free($this->xml); $this->test = str_repeat('xxxxx', 1000000); } public function test2() { $xml = xml_parser_create(); xml_set_character_data_handler($xml, array(&$this, 'handle_cdata')); xml_parser_free($xml); $this->test = str_repeat('xxxxx', 1000000); } public function test3() { $this->xml = xml_parser_create(); xml_set_character_data_handler($this->xml, array(&$this, 'handle_cdata')); xml_set_character_data_handler($this->xml, null); xml_parser_free($this->xml); $this->test = str_repeat('xxxxx', 1000000); } public function handle_cdata(&$parser, $data) { } } for($i = 1; $i < 100; $i++) { $object = new c(); $object->test(); unset($object); var_dump(memory_get_usage(true) / (1024*1024)); } ?> Note that test will show the memory leak, but test2 and test3 don't. I don't know if that's the same as bug #72793 or not. ------------------------------------------------------------------------ [2018-09-12 17:26:05] cmb@php.net > […] but I assume there is some reason why this is not feasible See bug #72793. ------------------------------------------------------------------------ [2018-09-12 16:01:37] ksours at internetbrands dot com Description: ------------ There is a note on the xml_set_object that you need to unset the xml resource variable to avoid memory leaks. This *really* needs to be more prominent. It's possible to hit this condition in code that does not call xml_set_object and that's not the obvious place to look for information on why xml_parser_free isn't working correctly. Recommend at a minimum that a similar not be placed on the xml_parser_free entry. (Ideally calling xml_parser_free *should* be sufficient to avoid holding that memory in place but I assume there is some reason why this is not feasible). ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=76874&edit=1