advanced_html_dom
advanced_html_dom copied to clipboard
Memory Leak
This library has a memory leak by design, see note [http://php.net/manual/en/language.oop5.decon.php#105368] I have been to be able to get a workaround at least temporary explicit calling a "destruct" method on AHTMLNode, AHTMLNodeList, AdvancedHtmlDom, AdvancedHtmlBase which are the class that I use, magic method __destruct is not calling see note above.
Can you post some code that demonstrates this?
On Sun, May 6, 2018 at 12:38 PM hashsup [email protected] wrote:
This library has a memory leak by design, see note [ http://php.net/manual/en/language.oop5.decon.php#105368] I have been to be able to get a workaround at least temporary explicit calling a "destruct" method on AHTMLNode, AHTMLNodeList, AdvancedHtmlDom, AdvancedHtmlBase which are the class that I use.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/monkeysuffrage/advanced_html_dom/issues/19, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1afL_QHkwYlsyydXZNenxxutC9vrpEks5tvn5ggaJpZM4Tz5Gk .
Sure, to avoid more complex example, please use a trivial html to load.
$n = 0; while($n++ < 20); { echo 'PRE LOAD '.(memory_get_peak_usage(TRUE) / 1024 / 1024)." Mib\n"; $object = file_get_html('load-a-1000Kib-html-file.html'); $object = NULL; echo 'POST LOAD '.(memory_get_peak_usage(TRUE) / 1024 / 1024)." Mib\n"; }
The issue is the design here:
` class AdvancedHtmlDom extends AdvancedHtmlBase{ public $xpath; public $root;
function __construct($html = null, $is_xml = false){ $this->doc = $this; `
That property $this->doc is the start of the end, it create a circular reference imposible to destroy even using __destructor [http://php.net/manual/en/language.oop5.decon.php#105368]
public function find($css, $index = null){ $xpath = CSS::xpath_for($css); if(!isset($this->doc) || !isset($this->doc->xpath)) return null; if(null === $index){ return new AHTMLNodeList($this->doc->xpath->query($xpath, $this->node), $this->doc); } else { $nl = $this->doc->xpath->query($xpath, $this->node); if($index < 0) $index = $nl->length + $index; $node = $nl->item($index); return $node ? new AHTMLNode($node, $this->doc) : null;
Passing the object $this->doc to AHTMLNode is not the correct way to do this, when we find this kind of issue is time to rethink the software design.
A workaround would be explicit call a destructor method for HTMLNode, AHTMLNodeList, AdvancedHtmlDom, AdvancedHtmlBase class and it should destroy the properties, for example the destructor of AdvancedHtmlDom is:
` public function destruct() {
$this->xpath = NULL;
$this->root = NULL;
$this->dom = NULL;
$this->doc = NULL;
unset($this->xpath);
unset($this->root);
unset($this->dom);
unset($this->doc);
} `
That sounds good to me. I don't see the issue when I run your code but I'm happy to accept a pull request.
On Sun, May 6, 2018 at 3:52 PM hashsup [email protected] wrote:
Sure, to avoid more complex example, please use a trivial html to load.
$n = 0; while($n++ < 20); { echo 'PRE LOAD '.(memory_get_peak_usage(TRUE) / 1024 / 1024)." Mib\n"; $object = file_get_html('load-a-1000Kib-html-file.html'); $object = NULL; echo 'POST LOAD '.(memory_get_peak_usage(TRUE) / 1024 / 1024)." Mib\n"; }
The issue is the design here:
` class AdvancedHtmlDom extends AdvancedHtmlBase{ public $xpath; public $root;
function __construct($html = null, $is_xml = false){ $this->doc = $this; `
That property $this->doc is the start of the end, it create a circular reference imposible to destroy even using __destructor [ http://php.net/manual/en/language.oop5.decon.php#105368]
public function find($css, $index = null){ $xpath = CSS::xpath_for($css); if(!isset($this->doc) || !isset($this->doc->xpath)) return null; if(null === $index){ return new AHTMLNodeList($this->doc->xpath->query($xpath, $this->node), $this->doc); } else { $nl = $this->doc->xpath->query($xpath, $this->node); if($index < 0) $index = $nl->length + $index; $node = $nl->item($index); return $node ? new AHTMLNode($node, $this->doc) : null; Passing the object $this->doc to AHTMLNode is not the correct way to do this, when we find this kind of issue is time to rethink the software design.
A workaround would be explicit call a destructor method for HTMLNode, AHTMLNodeList, AdvancedHtmlDom, AdvancedHtmlBase class and it should destroy the properties, for example the destructor of AdvancedHtmlDom is:
` public function destruct() {
$this->xpath = NULL; $this->root = NULL; $this->dom = NULL; $this->doc = NULL;
unset($this->xpath); unset($this->root); unset($this->dom); unset($this->doc);
} `
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/monkeysuffrage/advanced_html_dom/issues/19#issuecomment-386861157, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1afOqmALbKBWlyXKTFe36oYONYhCnLks5tvquzgaJpZM4Tz5Gk .
Sorry by delay, I created the pull request, note that it's an ugly temporary solution, but works, I have script working for days without issue.