After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 303682 - Memory leak in xmlNodeSetContent?
Memory leak in xmlNodeSetContent?
Status: VERIFIED FIXED
Product: libxml2
Classification: Platform
Component: general
2.6.19
Other All
: Normal major
: ---
Assigned To: Daniel Veillard
libxml QA maintainers
Depends on:
Blocks:
 
 
Reported: 2005-05-10 15:08 UTC by Malcolm Rowe
Modified: 2009-08-15 18:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Testcase (577 bytes, text/plain)
2005-05-10 15:11 UTC, Malcolm Rowe
Details

Description Malcolm Rowe 2005-05-10 15:08:39 UTC
Please describe the problem:
At least for #text nodes, xmlNodeSetContent() appears not to free the previous
content value, resulting in a memory leak.

Alternatively, I've misunderstood the API.

The problem appears to be an inversion in a test that determines whether
(xmlNodePtr)->content should be freed:

tree.c:5085 (xmlNodeSetContent)

if (cur->content != NULL) {
    if (!((cur->doc != NULL) && (cur->doc->dict != NULL) &&
        (!xmlDictOwns(cur->doc->dict, cur->content))))
        xmlFree(cur->content);
} 

Here, the intention seems to be that, providing ->doc and ->doc->dict are
non-NULL, we should first check whether the content is owned by the dictionary.
I assume (though I'm not completely sure) that this is to prevent us from
freeing interned strings.

However, the sense of the xmlDictOwns() test appears to be wrong. If the
dictionary does not own the string (and so we presumably _do_ want to free it),
xmlDictOwns() will return false, the expression as a whole will evaluate to
false, and we will _not_ free the string.

I'll attach a testcase demonstrating the problem.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Malcolm Rowe 2005-05-10 15:11:17 UTC
Created attachment 46293 [details]
Testcase

Valgrind output:

==12853== Memcheck, a memory error detector for x86-linux.
==12853== Copyright (C) 2002-2005, and GNU GPL'd, by Julian Seward et al.
==12853== Using valgrind-2.4.0, a program supervision framework for x86-linux.
==12853== Copyright (C) 2000-2005, and GNU GPL'd, by Julian Seward et al.
==12853== For more details, rerun with: -v
==12853==
==12853==
==12853== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 23 from 2)
==12853== malloc/free: in use at exit: 10 bytes in 1 blocks.
==12853== malloc/free: 46 allocs, 45 frees, 17147 bytes allocated.
==12853== For counts of detected errors, rerun with: -v
==12853== searching for pointers to 1 not-freed blocks.
==12853== checked 389120 bytes.
==12853==
==12853== 10 bytes in 1 blocks are definitely lost in loss record 1 of 1
==12853==    at 0x1B9022E8: malloc (vg_replace_malloc.c:130)
==12853==    by 0x1B9986D5: xmlStrndup (in /usr/lib/libxml2.so.2.6.19)
==12853==    by 0x1B9E1518: (within /usr/lib/libxml2.so.2.6.19)
==12853==    by 0x1B9E2539: xmlSAX2Characters (in /usr/lib/libxml2.so.2.6.19)
==12853==    by 0x1B934A0B: xmlParseCharData (in /usr/lib/libxml2.so.2.6.19)
==12853==    by 0x1B93E370: xmlParseContent (in /usr/lib/libxml2.so.2.6.19)
==12853==    by 0x1B93E728: xmlParseElement (in /usr/lib/libxml2.so.2.6.19)
==12853==    by 0x1B93FA62: xmlParseDocument (in /usr/lib/libxml2.so.2.6.19)
==12853==    by 0x1B944EA7: (within /usr/lib/libxml2.so.2.6.19)
==12853==    by 0x8048609: main (in /home/malcolm/blog/tools/memleak)
==12853==
==12853== LEAK SUMMARY:
==12853==    definitely lost: 10 bytes in 1 blocks.
==12853==      possibly lost: 0 bytes in 0 blocks.
==12853==    still reachable: 0 bytes in 0 blocks.
==12853==	  suppressed: 0 bytes in 0 blocks.
==12853== Reachable blocks (those to which a pointer was found) are not shown.
==12853== To see them, rerun with: --show-reachable=yes
Comment 2 Malcolm Rowe 2005-05-10 15:15:16 UTC
The bug (if it is a bug) appears to have been introduced in tree.c v1.324, which
was itself a fix for a memory leak. However, I don't fully understand the
original fix, so I don't know whether I'm on the right track or not.

tree.c v1.324:
* parser.c: fixed the leak reported by Volker Roth on the list
* test/ent10 result//ent10*: added a specific test for the problem
Daniel

See also:
http://cvs.gnome.org/viewcvs/libxml2/tree.c?r1=1.323&r2=1.324
http://mail.gnome.org/archives/xml/2004-October/msg00085.html
http://mail.gnome.org/archives/xml/2004-October/msg00086.html
Comment 3 Daniel Veillard 2005-05-10 15:29:16 UTC
 Doohh, right ... double negation in the test, this is a real bug !
I fixed the code, added xmlMemoryDump(); to tst.c and both valgrind and
the memory dump indicate there is no leak anymore. So I fixed it in CVS

  thanks a lot of the excellent report !

Daniel
Comment 4 Daniel Veillard 2005-09-05 08:59:50 UTC
This should be closed by release of libxml2-2.6.21,

  thanks,

Daniel