Re: [xml] [patch] make libxml2 slightly more thread-friendly

[Daniel Veillard <veillard redhat com>]

On Mon, Feb 12, 2007 at 06:37:25AM -0600, Ted Phelps wrote:
> As documented here at the following URL, applications which use libxml2
> are required to 'call xmlInitParser() in the "main" thread before using
> any of the libxml2 API'.
>
>     http://xmlsoft.org/threads.html
>
> That's fine for applications, but less helpful for modules which are
> loaded into larger application (think apache or the X11 server).  In the
> latter case, each module which uses libxml2 will need to call
> xmlInitParser() since it won't be able to determine whether or not
> another module has done so.  If the larger application happens to run
> these modules in separate threads then the test at the beginning of
> xmlParserInit is insufficient to ensure that the initialization is
> performed exactly once.  Under some circumstances it's possible for a
> thread to hang waiting for a mutex to be unlocked which will never be
> unlocked (see http://bugzilla.gnome.org/show_bug.cgi?id=406099).
>
> The following patch (also available as an attachment in the above bug
> report) adds some extra checking to xmlInitParser() to avoid these
> problems with zero extra overhead for the non-initialization case.
> Please consider adding it to libxml2.  Of course I'm happy to rework it
> in whatever way the maintainers see fit.

  Okay, fixing the problem sounds fine of course, but I have a couple of 
issues with the patch:

   - all thread implementation specific code in libxml2 is contained in
     threads.c module, that patch breal that containment, I would really
     prefer if this code was kept in threads.c
   - it looks to me that this code would break if libxml2 is configured
     without thread support (configure --without-threads)
   - it uses the keyword 'volatile' which is not used anywhere else in 
     libxml2, I would prefer to avoid it if possible to maximize portability
     (plus its semantic is IMHO rather fuzzy)

I'm not sure how to best achieve those improvements, as this code really
mix the xmlInitParser with really thread-specific code. Also we had trouble
in the past with the xmlInitGlobals()/xmlInitThreads()/xmlInitMemory() ordering
of initializers, the interraction are relatively complex to debug I hope
the new code doesn't have side effect, that seems true as it uses direct
memory allocations, it's just something to keep in mind.

  Ted, do you think you could try to rewrite the patch to address the 3 issues
listed before, this may require separing the xmlInitParser into one routine
when thread are not compiled in, and one in threads.c when thread support is
compiled in (but that's just a suggestion).

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/