Re: [xml] win32 thread safety patch

[Igor Zlatkovic <igor stud fh-frankfurt de>]

> 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
>
>
> #ifdef LIBXML_THREAD_ENABLED
> #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
> problems!

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
> would
> > 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
> things.
> 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?

Igor