Bug 554903

Summary: Please print a warning (or abort) if xmlCleanupParser() is called twice
Product: [Fedora] Fedora Reporter: Lennart Poettering <lpoetter>
Component: libxml2Assignee: Daniel Veillard <veillard>
Status: CLOSED WONTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: 14CC: mschmidt, nsoranzo, veillard
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-08-16 21:18:42 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
reproducer to show xmlCleanupParser() is never safe to call twice none

Description Lennart Poettering 2010-01-12 23:13:18 UTC
Quite a few programs seem to call xmlCleanupParser() at inappropriate places and more than once. This can have the effect that TLS fields of other libraries are freed which results in a big disaster.

For now it seems at least empathy and abiword do this wrong, but a quick google code search reveals that inkscape and dia call this function where they shouldn't, too. I kinda extrapolate from that that there are many more who misuse that, and due to that I'd like to ask that a check is added to that function to warn about this misuse on the second call.

Also see:

https://bugzilla.redhat.com/show_bug.cgi?id=532307

Comment 1 Daniel Veillard 2010-01-13 10:18:42 UTC
http://xmlsoft.org/html/libxml-parser.html#xmlCleanupParser

"This function name is somewhat misleading. It does not clean up parser state, it cleans up memory allocated by the library itself. It is a cleanup function for the XML library. It tries to reclaim all related global memory allocated for the library processing. It doesn't deallocate any document related memory. One should call xmlCleanupParser() only when the process has finished using the library and all XML/HTML documents built with it. See also xmlInitParser() which has the opposite function of preparing the library for operations. WARNING: if your application is multithreaded or has plugin support calling this may crash the application if another thread or a plugin is still using libxml2. It's sometimes very hard to guess if libxml2 is in use in the application, some libraries or plugins may use it without notice. In case of doubt abstain from calling this function or do it just before calling exit() to avoid leak reports from valgrind !"

How is that not clear enough ?

Daniel

Comment 2 Daniel Veillard 2010-01-13 10:38:40 UTC
Note that an application could perfectly call xmlCleanupParser() multiple
time, if it controls the fact that it's the single user of the library
and uses libxml2 at very specific moments. So I'm still unclear about the
warning there, though for the majority of use it's likely to be a real problem
but you can't garantee it.

Daniel

Comment 3 Lennart Poettering 2010-01-13 14:23:18 UTC
(In reply to comment #1)
> http://xmlsoft.org/html/libxml-parser.html#xmlCleanupParser
> 
> How is that not clear enough ?

It's a simple fact that nobody has read that, even if they should. They probably just did a copy/paste from some other code which already did it wrong.

A google code search or a debian source code grep shows that really about nobody got this right. empathy, abiword, inkscape, dia, you name it.

There is no doubt that that those apps are broken and need to be fixed. The problem is that it is really hard to track them all down. It would make things a lot easier if libxml2 would by itself warn about this misuse. It's a trivial patch and would make it easy to fix the apps in question. This bug is not about something to fix in libxml2, it's just about something that would make it easier to track down those who misuse it.

Also, in times where people use gcc destructors and threaded code I believe it is practically impossible to call xmlCleanupParser() properly, in everything but the most trivial code. However, if you want to allow calling it, wouldn't it be possible to at least makeit idempotent? Or count how often it was called and decrease the counter if the library is reinitialized again?

Comment 4 Michal Schmidt 2010-03-30 09:05:55 UTC
(In reply to comment #2)
> Note that an application could perfectly call xmlCleanupParser() multiple
> time, if it controls the fact that it's the single user of the library
> and uses libxml2 at very specific moments.

Daniel,
you are wrong here. Even if the program is the only user of libxml2, it can still get into trouble by calling xmlCleanupParser() multiple times. I'll attach a trivial reproducer.

Comment 5 Michal Schmidt 2010-03-30 09:06:51 UTC
Created attachment 403444 [details]
reproducer to show xmlCleanupParser() is never safe to call twice

Comment 6 Michal Schmidt 2010-03-30 09:10:02 UTC
Build it with:
gcc -Wall -g -lpthread `pkg-config --libs --cflags libxml-2.0` -o xml2test xml2test.c

$ ./xml2test 
Failed to set the thread-specific value: Invalid argument

Comment 7 Michal Schmidt 2010-06-25 15:14:30 UTC
Daniel, any comments about the single-threaded reproducer in comment 5?

Comment 8 Bug Zapper 2010-11-04 01:07:06 UTC
This message is a reminder that Fedora 12 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 12.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '12'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 12's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 12 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

Comment 9 Fedora End Of Life 2012-08-16 21:18:44 UTC
This message is a notice that Fedora 14 is now at end of life. Fedora 
has stopped maintaining and issuing updates for Fedora 14. It is 
Fedora's policy to close all bug reports from releases that are no 
longer maintained.  At this time, all open bugs with a Fedora 'version'
of '14' have been closed as WONTFIX.

(Please note: Our normal process is to give advanced warning of this 
occurring, but we forgot to do that. A thousand apologies.)

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, feel free to reopen 
this bug and simply change the 'version' to a later Fedora version.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we were unable to fix it before Fedora 14 reached end of life. If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora, you are encouraged to click on 
"Clone This Bug" (top right of this page) and open it against that 
version of Fedora.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping