php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #76874 xml_parser_free() should never leak memory
Submitted: 2018-09-12 16:01 UTC Modified: 2019-05-09 09:19 UTC
Votes:3
Avg. Score:3.7 ± 1.9
Reproduced:2 of 2 (100.0%)
Same Version:2 (100.0%)
Same OS:1 (50.0%)
From: ksours at internetbrands dot com Assigned: nikic (profile)
Status: Closed Package: *XML functions
PHP Version: 7.3.0beta3 OS: *
Private report: No CVE-ID: None
 [2018-09-12 16:01 UTC] 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).


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-09-12 17:26 UTC] cmb@php.net
-Status: Open +Status: Verified -Assigned To: +Assigned To: cmb
 [2018-09-12 17:26 UTC] cmb@php.net
> […] but I assume there is some reason why this is not feasible

See bug #72793.
 [2018-09-12 20:42 UTC] 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-13 13:14 UTC] 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-14 12:09 UTC] 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-14 12:11 UTC] cmb@php.net
-Summary: Documentation of memory leak in Xml Parser +Summary: xml_parser_free() should never leak memory -Package: Documentation problem +Package: *XML functions -Operating System: Windows +Operating System: * -Assigned To: cmb +Assigned To:
 [2018-09-14 12:11 UTC] cmb@php.net
The docs should be okay now, so changing to the actual bug.
 [2018-09-17 09:06 UTC] nikic@php.net
-Status: Verified +Status: Assigned -Assigned To: +Assigned To: nikic
 [2018-09-17 09:06 UTC] nikic@php.net
WIP patch up at https://github.com/php/php-src/pull/3526.
 [2019-05-09 09:19 UTC] nikic@php.net
-Status: Assigned +Status: Closed
 [2019-05-09 09:19 UTC] nikic@php.net
ext/xml has been migrated to use objects in PHP 8, so the issue will be resolved there.
 [2020-02-07 06:05 UTC] phpdocbot@php.net
Automatic comment on behalf of cmb
Revision: http://git.php.net/?p=doc/en.git;a=commit;h=4279863b96f74295fb6bd2d44e34270a71fb9537
Log: Fix #76874: Documentation of memory leak in Xml Parser
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Wed Dec 04 00:01:32 2024 UTC