Remove risk of lockup in dictionary initialization

Reported by Petr Sumbera <[email protected]> Two threads entering xmlInitializeDict concurently could lead to a lockup due to multiple initializations of the lock used. To avoid this problem move this to a new private function called from xmlOnceInit() and deprecate the old initalizer. Since threaded programs must call xmlInitParser() and this will lead to dereference of private data and the call to xmlOnceInit() guaranteed to be unique this should be safe now.
DaDummy · Apr 5, 2013 · 5fe9e9e · 5fe9e9e
1 parent bf4a8f0
commit 5fe9e9e
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 6 deletions.
diff --git a/dict.c b/dict.c
@@ -151,13 +151,28 @@ static unsigned int rand_seed = 0;
  * xmlInitializeDict:
  *
  * Do the dictionary mutex initialization.
- * this function is not thread safe, initialization should
- * preferably be done once at startup
+ * this function is deprecated
  *
  * Returns 0 if initialization was already done, and 1 if that
  * call led to the initialization
  */
 int xmlInitializeDict(void) {
+    return(0);
+}
+
+/**
+ * __xmlInitializeDict:
+ *
+ * This function is not public
+ * Do the dictionary mutex initialization.
+ * this function is not thread safe, initialization should
+ * normally be done once at setup when called from xmlOnceInit()
+ * we may also land in this code if thread support is not compiled in
+ *
+ * Returns 0 if initialization was already done, and 1 if that
+ * call led to the initialization
+ */
+int __xmlInitializeDict(void) {
     if (xmlDictInitialized)
         return(1);
 
@@ -183,7 +198,7 @@ int __xmlRandom(void) {
     int ret;
 
     if (xmlDictInitialized == 0)
-        xmlInitializeDict();
+        __xmlInitializeDict();
 
     xmlRMutexLock(xmlDictMutex);
 #ifdef HAVE_RAND_R
@@ -522,7 +537,7 @@ xmlDictCreate(void) {
     xmlDictPtr dict;
 
     if (!xmlDictInitialized)
-        if (!xmlInitializeDict())
+        if (!__xmlInitializeDict())
             return(NULL);
 
 #ifdef DICT_DEBUG_PATTERNS
@@ -590,7 +605,7 @@ xmlDictCreateSub(xmlDictPtr sub) {
 int
 xmlDictReference(xmlDictPtr dict) {
     if (!xmlDictInitialized)
-        if (!xmlInitializeDict())
+        if (!__xmlInitializeDict())
             return(-1);
 
     if (dict == NULL) return -1;
@@ -754,7 +769,7 @@ xmlDictFree(xmlDictPtr dict) {
 	return;
 
     if (!xmlDictInitialized)
-        if (!xmlInitializeDict())
+        if (!__xmlInitializeDict())
             return;
 
     /* decrement the counter, it may be shared by a parser and docs */

diff --git a/libxml.h b/libxml.h
@@ -84,6 +84,8 @@ void __xmlGlobalInitMutexLock(void);
 void __xmlGlobalInitMutexUnlock(void);
 void __xmlGlobalInitMutexDestroy(void);
 
+int __xmlInitializeDict(void);
+
 #if defined(HAVE_RAND) && defined(HAVE_SRAND) && defined(HAVE_TIME)
 /*
  * internal thread safe random function

diff --git a/threads.c b/threads.c
@@ -954,13 +954,15 @@ xmlOnceInit(void)
 #ifdef HAVE_PTHREAD_H
     (void) pthread_key_create(&globalkey, xmlFreeGlobalState);
     mainthread = pthread_self();
+    __xmlInitializeDict();
 #elif defined(HAVE_WIN32_THREADS)
     if (!run_once.done) {
         if (InterlockedIncrement(&run_once.control) == 1) {
 #if !defined(HAVE_COMPILER_TLS)
             globalkey = TlsAlloc();
 #endif
             mainthread = GetCurrentThreadId();
+	    __xmlInitializeDict();
             run_once.done = 1;
         } else {
             /* Another thread is working; give up our slice and
@@ -974,6 +976,7 @@ xmlOnceInit(void)
         globalkey = tls_allocate();
         tls_set(globalkey, NULL);
         mainthread = find_thread(NULL);
+	__xmlInitializeDict();
     } else
         atomic_add(&run_once_init, -1);
 #endif