Re: [xml] libxml2 thread safety (SUMMARY)

[Daniel Veillard <veillard redhat com>]

On Thu, Oct 04, 2001 at 10:13:38AM +0100, Gary Pennington wrote:
> Here's the appropriate snippet (relating to a single variable 
> xmlDoValidityCheckingDefaultValue for puposes of illustration, there 
> would be one of these for each global that was modified) from the header 
> file to show how it works:
>
> #if defined(_REENTRANT) || (_POSIX_C_SOURCE - 0 >= 199506L)

What I dislike in this solution is that we end-up with 2 different libraries
one when one compiles with _REENTRANT and one without this. From a packaging
and maintainance point of view this is no fun at all.

> extern int *__xmlDoValidityCheckingDefaultValue();
> #define xmlDoValidityCheckingDefaultValue \
> (*(__xmlDoValidityCheckingDefaultValue()))
> #else
> extern int xmlDoValidityCheckingDefaultValue;
> #endif
>
> Any new code compiled would either be threaded or non-threaded. It would 
> pick up the macro definition if threaded, the standard extern symbol if 
> not. Existing code would still use the standard extern symbol and would 
> thus behave as non-threaded code.
>
> The implementation of __xmlDoValidityCheckingDefaultValue() shows how 
> the value of the TSD is manipulated
>
> #undef  xmlDoValidityCheckingDefaultValue
>
> int xmlDoValidityCheckingDefaultValue = 0;
>
> int *
> __xmlDoValidityCheckingDefaultValue()
> {
>         struct glob_struct *globalval;
>
>         if (keyonce == 0) {
>                 (void) pthread_mutex_lock(&keylock);
>                 if (keyonce == 0) {
>                         keyonce++;
>                         (void) pthread_key_create(&globalkey, tsd_free);
>                 }
>                 (void) pthread_mutex_unlock(&keylock);
>         }
>         if ((globalval = (struct glob_struct 
> *)pthread_getspecific(globalkey))
>             == NULL) {
>                 struct glob_struct *tsd = alloc_glob_struct();
>                 pthread_setspecific(globalkey, tsd);
>                 return (&tsd->xmlDoValidityCheckingDefaultValue);
>         } else
>                 return (&globalval->xmlDoValidityCheckingDefaultValue);
> }

  Okay, I would not call this beautiful but it is acceptable if contained
to a limited area. There is also 2 places where I would need to take a lock
in other places in the code. I suggest the following:
    - sepearate the global state definition in a state.h header
    - move all this code and definitions of the global variables in
      a state.c module
    - export the locking as a libxml function and data to help portability,
      this is not a full encapsulation of a thread system, just the
      create/lock/unlock/destroy of a lock since it will be used in other
      places in libxml (and possibly libxslt too).

> I really think that the above is only a temporary measure until a 
> "proper" re-working of libxml (i.e. libxml3) can be performed. It's a 
> solution that promotes backwards-compatibility, however it's not as 
> clean as doing the reworking. Some of the work required to remove global 
> state would be performed which would help in the migration to libxml3 
> and that would mean phased implementation of the threading support, 
> which is either a good or a bad thing depending on your point of view. 

  This is fine by me at least.

> We could decide that this is not worth doing and that we should just 
> wait for libxml3 and do a non-backwards compatible reworking. I have 
> some (limited) time to work on making this happen, but I'd be happy to 
> defer and wait to do a proper reworking for libxml2 if I knew what the 
> time frame for that project was.

  Go ahead. I will integrate it, I probably won't make it the default
(i.e. it will require a configure flag) but once the capacity will be there
we will get more feedback.

> Anyway, I have a series of test programs using the above partial 
> implementation which work fine single threaded - I still need to write a 
> more comprehensive mt-driver (I've run through the testall target) and 
> the main (technically) tricky bits remaining are :
> 1. Deciding exactly which global symbols go into the global symbol 
> structure. All global variables or only those we expect to be modified?

  I would go for the latter. For example the predefined entities list 
is global and should be shared by all threads, I would not put it in
the structure. Other variables are harder, like the Catalog ones, it
could make sense to maintain them per thread but they are costly to build,
this is not simple to decide.

> 2. Working out the appropriate configure incantation to make thread 
> support optional even if detected on the platform. (If we decide we want 
> this to be an option)

   definitely.

Non-technically, writing the docs will be extremely important too.

Daniel

-- 
Daniel Veillard      | Red Hat Network http://redhat.com/products/network/
veillard redhat com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/