|  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #61797 DOMNode::appendChild doesn't increment refcount
Submitted: 2012-04-21 10:37 UTC Modified: 2019-09-22 22:10 UTC
Avg. Score:3.8 ± 1.2
Reproduced:4 of 4 (100.0%)
Same Version:0 (0.0%)
Same OS:4 (100.0%)
From: kuba dot brecka at gmail dot com Assigned: beberlei (profile)
Status: Wont fix Package: DOM XML related
PHP Version: 5.4.0 OS: Windows, Linux
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Bug Type:
From: kuba dot brecka at gmail dot com
New email:
PHP Version: OS:


 [2012-04-21 10:37 UTC] kuba dot brecka at gmail dot com
When I create a DOM element and I add it via DOMDocument->appendChild, it seems 
that the refcount to that object is not increased. This causes the appended child 
to be deallocated if there are no other references. It also causes some funky 
behaviour, because the freed object can still be accesssed (doesn't) crash, but 
it seems to be of different type. Don't know the cause, maybe the memory gets 
either reused to create another object.

I created a simple test case that proves and reproduces this buggy behaviour, 
AFAIK all PHP versions are affected, I tested 5.4.0 on Windows.

Test script:
class MyElement extends DOMElement { }

// #1 - okay
$dom = new DOMDocument();
$e = new MyElement("e");
echo get_class($dom->childNodes->item(0)) . "\n";

// #2 - wrong
$dom = new DOMDocument();
$dom->appendChild(new MyElement("e"));
echo get_class($dom->childNodes->item(0)) . "\n";

// #3 - wrong
$dom = new DOMDocument();
$e = new MyElement("e");
$e = null;
echo get_class($dom->childNodes->item(0)) . "\n";

Expected result:

Actual result:


dom_node_append_child.patch (last revision 2013-12-02 19:55 UTC by

Add a Patch

Pull Requests

Add a Pull Request


AllCommentsChangesGit/SVN commitsRelated reports
 [2012-04-26 13:27 UTC]
Shouldn't you just use registerNodeClass() ?

See for a modified script
 [2012-04-26 13:27 UTC]
-Status: Open +Status: Feedback
 [2012-04-26 13:46 UTC] kuba dot brecka at gmail dot com
-Status: Feedback +Status: Open
 [2012-04-26 13:46 UTC] kuba dot brecka at gmail dot com
Well, registerNodeClass solves it. But I still believe that without it, the 
behaviour should not depend on the refcount of the object you are trying to use in 
DOM. It should at least emit a warning or an exception, that you are using an 
object, which has not beed registered.

Anyway, the need to use registerNodeClass should be mentioned in the docs, at 
least in appendChild.
 [2013-12-02 16:35 UTC]
-Type: Bug +Type: Documentation Problem
 [2013-12-02 19:43 UTC]
The following patch has been added/updated:

Patch Name: dom_node_append_child.patch
Revision:   1386013405
 [2013-12-02 19:44 UTC]
That is clearly wrong ... patch attached ... review please, no need to wait any longer ...
 [2013-12-02 19:55 UTC]
The following patch has been added/updated:

Patch Name: dom_node_append_child.patch
Revision:   1386014157
 [2013-12-02 19:57 UTC]
It is obvious from tests that this behaviour is expected, that doesn't make it any more correct ... it might create compatibility issues if fixed ... I don't know ... patch updated to fix the tests
 [2013-12-03 00:26 UTC]
-Type: Documentation Problem +Type: Bug
 [2013-12-03 00:26 UTC]
This is not a doc issue.. Reclassified as a bug.
 [2013-12-03 07:37 UTC]
-Assigned To: +Assigned To: rrichards
 [2013-12-03 07:37 UTC]
Your patch reads like a bug fix, but reading it in context with the source it is not definitely a bug. Why would you suggest registerNodeClass() exists?

If we change that, then obviously only for master.
But let's first try to reach an author for comment.

 [2013-12-03 07:45 UTC]
The non-standard registerNodeClass exists to service the createElement functionality of the DOM, there is no other logical reason for it's existence.

The patch doesn't change that, it enforces reference counting, which is required, everywhere.
 [2015-05-17 16:57 UTC]
It appears to me that this is not a reference counting issue; at
least the additional zval_copy_ctor() leaks memory with a recent
master on Windows.
 [2017-10-24 06:14 UTC]
-Status: Assigned +Status: Open -Assigned To: rrichards +Assigned To:
 [2019-09-22 22:10 UTC]
-Status: Open +Status: Wont fix
 [2019-09-22 22:10 UTC]
This is not a bug. DOMDocument guarantees you the return of a DOMElement from a childNode. If you pass a subclass of DOMElement, then you must register is using "registerNodeClass". Not doing so will only guarantee you a DOMElement, depending if original element fell out of scope.
 [2019-09-22 22:10 UTC]
-Assigned To: +Assigned To: beberlei
 [2019-09-22 22:33 UTC]
In addition, you should see childNodes, documentElement and other properties holding DOMNode and subclasses as "magic" properties using __get, that return new instances as they like based on the underlying libxml structure.

ext/dom makes *no* guarantee to keep references matching to individual libxml nodes. This strategy is also used to keep memory usage low(er).
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Jun 25 21:01:29 2024 UTC