After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 406099 - xmlInitializeGlobalState not threadsafe
xmlInitializeGlobalState not threadsafe
Status: RESOLVED WONTFIX
Product: libxml2
Classification: Platform
Component: general
2.6.27
Other All
: Normal normal
: ---
Assigned To: Daniel Veillard
libxml QA maintainers
Depends on:
Blocks:
 
 
Reported: 2007-02-09 14:27 UTC by Ted Phelps
Modified: 2007-02-12 12:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Simple program to demonstrate this bug (2.17 KB, text/plain)
2007-02-09 14:36 UTC, Ted Phelps
  Details
proposed patch (3.57 KB, patch)
2007-02-09 18:22 UTC, Ted Phelps
none Details | Review
improved patch proposal (3.66 KB, patch)
2007-02-12 12:16 UTC, Ted Phelps
none Details | Review

Description Ted Phelps 2007-02-09 14:27:38 UTC
Please describe the problem:
The function xmlInitializeGlobalState suffers from a race condition which can result in a thread blocking forever.  Here are the relevant bits of code:

47: void xmlInitGlobals(void)
48: {
49:     xmlThrDefMutex = xmlNewMutex();
50: }

483: void
484: xmlInitializeGlobalState(xmlGlobalStatePtr gs)
485: {
         ...
491: 
492:     /*
493:      * Perform initialization as required by libxml
494:      */
495:     if (xmlThrDefMutex == NULL)
496:         xmlInitGlobals();
497: 
498:     xmlMutexLock(xmlThrDefMutex);
         ...
554:     xmlMutexUnlock(xmlThrDefMutex);
555: }

Consider the following 3-thread scenario:

thr-1, 495: xmlThrDefMutex == NULL
thr-2, 495: xmlThrDefMutex == NULL
thr-1,  49: xmlThrDefMutex <- [mutex-1]
thr-1, 498: xmlMutexLock([mutex-1]) -> granted
thr-3, 495: xmlThrDefMutex != NULL
thr-3, 498: xmlMutexLock([mutex-1]) -> blocks
thr-2,  49: xmlThrDefMutex <- [mutex-2]
thr-3, 554: xmlMutexUnlock([mutex-2])

In this scenario, nothing will ever unlock [mutex-1], although [mutex-2] is unlocked one time more than it is locked.  That this is what's happening is confirmed by the contents of xmlThrDefMutex in a hung test_threaded_clients:

(gdb) print *xmlThrDefMutex
$3 = {
  lock = {
    __data = {
      __lock = 0, 
      __count = 0, 
      __owner = 0, 
      __nusers = 4294967295, 
      __kind = 0, 
      __spins = 0, 
      __list = {
        __prev = 0x0, 
        __next = 0x0
      }
    }, 
    __size = '\0' <repeats 12 times>, "����", '\0' <repeats 23 times>, 
    __align = 0
  }
}

Note that the value of __nusers is (unsigned int)-1.  I've written a short test program to verify that the Linux posix threads implementation permits a mutex to be locked once and unlocked twice and that doing so results in __nusers of 4294967295.



Steps to reproduce:
Call xmlParseDTD from at least three threads simultaneously.  Using more threads should increase the likelihood of this race occurring.

Actual results:
One or more threads will block indefinitely calling xmlMutexLock(xmlThrDefMutex).

Expected results:
No deadlock

Does this happen every time?
Unfortunately no.

Other information:
Comment 1 Ted Phelps 2007-02-09 14:36:53 UTC
Created attachment 82223 [details]
Simple program to demonstrate this bug

Compile with:
  gcc -pthread -o bug406099 -g -Wall -O2 -I/usr/include/libxml2 -pipe bug406099.c -lxml2

Run with:
  for X in `seq 1 1000` ; do bug406099; done

This hangs about 1 time in 20 for me on a dual 64-bit CPU box.  YMMV.

-Ted
Comment 2 Daniel Veillard 2007-02-09 15:06:38 UTC
Threaded applications must call xmlInitParser() in their main
thread beforte using libxml2. Documented there
   http://xmlsoft.org/threads.html
Since I know no way of avoiding this problem one way or another
and I guess your problem is ultimately due to the lack of initialization
it's a WONTFIX (unless you can explain why xmlInitGlobals() wasn't called
if you did the proper initialization)

Daniel
Comment 3 Ted Phelps 2007-02-09 15:16:02 UTC
Thanks for the speedy response.

Unfortunately, I cannot simply call xmlInitGlobals from the main application thread because I'm writing a library, not an application.  I could require that the application call my library's init function from its main thread, but I'd like for my library to be useable from applications like apache which dynamically load modules -- if one module uses my library and another uses libxml2 then we're going to run into initialization problems.

The way to resolve this issue with pthreads is to use pthread_once.  The way to resolve it in Windows is to use InterlockedCompareExchange.

If I were to provide a patch would you at least consider it?

Thanks,
-Ted
Comment 4 Daniel Veillard 2007-02-09 15:25:03 UTC
Sure, patches are looked at. Put it here and send it to the list for 
review,

Daniel
Comment 5 Ted Phelps 2007-02-09 18:22:50 UTC
Created attachment 82237 [details] [review]
proposed patch

Attached is a patch which ensures that the interesting parts of xmlInitParser are executed only once.  I've tested that it works on Linux and Windows.  Unfortunately I don't have access to a BeOS machine to verify that portion of the patch.
Comment 6 Ted Phelps 2007-02-09 18:31:21 UTC
Comment on attachment 82223 [details]
Simple program to demonstrate this bug

>#include <stdio.h>
>#include <stdlib.h>
>#include <stdint.h>
>#include <unistd.h>
>#include <string.h>
>#include <errno.h>
>#include <pthread.h>
>#include <libxml/globals.h>
>#include <libxml/tree.h>
>#include <libxml/entities.h>
>#include <libxml/parser.h>
>#include <libxml/parserInternals.h>
>#include <libxml/valid.h>
>
>#define NTHREADS 20
>
>pthread_mutex_t mutex;
>pthread_cond_t condvar;
>int pending, cycle;
>
>void *thread_main(void *arg)
>{
>    int err;
>
>    /* Wait for all threads to get started */
>    err = pthread_mutex_lock(&mutex);
>    if (err) {
>	perror("pthread_mutex_lock failed");
>	exit(1);
>    }
>
>    pending--;
>
>    if (pending == 0) {
>	cycle++;
>	pthread_cond_broadcast(&condvar);
>    } else {
>	while (cycle == 0) {
>	    err = pthread_cond_wait(&condvar, &mutex);
>	    if (err) {
>		perror("pthread_cond_wait failed");
>		exit(1);
>	    }
>	}
>    }
>
>    err = pthread_mutex_unlock(&mutex);
>    if (err) {
>	perror("pthread_mutex_unlock failed");
>    }
>
>    xmlInitParser();
>    return NULL;
>}
>
>int main(int argc, char *argv[])
>{
>    pthread_t threads[NTHREADS];
>    void *value;
>    int err, i;
>
>    /* Initialize a barrier */
>    err = pthread_mutex_init(&mutex, NULL);
>    if (err) {
>	perror("pthread_mutex_init failed");
>	exit(1);
>    }
>
>    err = pthread_cond_init(&condvar, NULL);
>    if (err) {
>	perror("pthread_cond_init failed");
>	exit(1);
>    }
>    pending = NTHREADS;
>    cycle = 0;
>
>    /* Start a bunch of threads */
>    for (i = 0; i < NTHREADS; i++) {
>	err = pthread_create(&threads[i], NULL, thread_main, (void *)(intptr_t)i);
>	if (err) {
>	    perror("pthread_create failed");
>	    exit(1);
>	}
>    }
>
>    for (i = 0; i < NTHREADS; i++) {
>	err = pthread_join(threads[i], &value);
>	if (err) {
>		perror("pthread_join failed");
>		exit(1);
>	}
>    }
>
>    err = pthread_cond_destroy(&condvar);
>    if (err) {
>	perror("pthread_cond_destroy failed");
>	exit(1);
>    }
>
>    err = pthread_mutex_destroy(&mutex);
>    if (err) {
>	perror("pthread_mutex_destroy failed");
>	exit(1);
>    }
>
>    putchar('.'); fflush(stdout);
>    exit(0);
>}
Comment 7 Ted Phelps 2007-02-12 12:16:38 UTC
Created attachment 82379 [details] [review]
improved patch proposal

This version of the patch fixes some obvious errors in the BeOS locking.