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

On Mon, Feb 12, 2007 at 10:18:03AM -0600, Ted Phelps wrote:

Hi Daniel,

As always, thank you for your speedy reply.

Daniel Veillard writes:
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

I thought you might.  I initially chose not to because I didn't want to
add new functions to the shared object, but I'm happy to do so if you
are.  Or do you have some mechanism to prevent functions from being
exported to the aplication?

  see libxml.h in same directory as the C sources, prefix the function names
by __ so even if it shows up in the shared object people are not tempted to
call them and dont export from include/libxml/*.h
   - 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 included the volatile keyword for the Windows code because the
declaration of InterlockedCompareExchangePointer indicates that the
first parameter should be of type 'PVOID volatile *'.  I believe that we
can safely leave this out and I have removed it from the patch below.

For the BeOS implementation, I have changed 'volatile' to vint32,
following the definition of atomic_add:

You may want to consider changing run_once_init to be a vint32 too.

  Hum, if you know you're in a specific environment where volatile is expected
then go ahead, just avoid it for the platform agnostic code :-)

I'm not sure how to best achieve those improvements, as this code
really mix the xmlInitParser with really thread-specific code.

I have shifted the thread-related code into threads.c, leaving only
calls to lock and unlock a global mutex.

  Looks good except the __ renaming and include file.

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.

I believe this patch should be immmune to such interactions.

  yes I believe so too now :-)

No worries.  Here's a second attempt.  Please have a look and let me
know if you'd like me to try a different approach or make additional

  Would you be so kind to make the last changes for the function name
and include, then I guess that should be commited,

  thanks !


Red Hat Virtualization group
Daniel Veillard      | virtualization library
veillard redhat com  | libxml GNOME XML XSLT toolkit | Rpmfind RPM search engine

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]