Re: [xml] win32 thread safety patch

Ok. The patch looks fine to me - (I was checking for cygwin
interactions). It's all #ifdef'd safely, except for the

static void
xmlFreeGlobalState(void *state)

and related bits that are only protected with

#ifndef _MSC_VER

Which will break (AFAICT) on unix builds - that protection should
include the HAVE_WIN32_THREADS test.

No, xmlFreeGlobalState is used on Unix. It is precisely the destructor
function which is specified in the call to pthread_key_create.

However some error tests are missing: for example TlsAlloc may not
succeed, but it's not tested for failure, which could lead to serious

Failure tests are missing everywhere in that part, that is true :-) If these
discussions don't cancel this whole solution, we will fix this. For now, the
chance that this function fails is so small, it lets one sleep in peace.

a) This is not any simpler than what Serguei did, but is a solution. I
welcome a patch.

It may not be simpler but it is more efficient :]. The overhead of an
extra thread per TLS value per thread is potentially large. I don't know
how many TLS values libXML uses, but if it used say 5, and a user app
accessed libxml via 5 threads we'd end up with 25 idle threads!

A patch is still welcome.

Secondly the pthread_key functionality requires (in libxml only) 3
1) A thread wrapper function. This function is called instead of the
thread to be started - so that returning from the thread function
invokes the cleanup code.
2) An ExitThread replacement macro (to prevent ExitThread() being called
which would shortcircuit the cleanup code.
3) The pthread_key_create, pthread_getspecific and pthread_setspecific
functions and the cleanupcode.

Hm... I am not sure if I understood this well. What would you wrap under 1),
the thread execution routine, or _beginthread function?

It seems that you assume libxml is the only library a program uses. Imagine
what would happen if a client app would use two libraries which would both
solve the threading question the way you proposed.

Looking at the code you sent, either me or you slightly misunderstood the
basics of TSD. A TSD key is allocated per process, not per thread. Libxml
needs exactly one TSD key per process. The client application may allocate
additional keys if it needs to store per-thread data. All threads of a
process can use the same key when setting and getting their TSD data.

Am I missing something?


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