Re: [xml] Re: Thread-safety of XSLT contexts with shared dictionaries

[Gary Coady <gary lyranthe org>]

Daniel Veillard wrote:
> > On Thu, Jan 20, 2005 at 08:37:18PM +0000, Gary Coady wrote:
> >
> > > + void
> > > + xmlDictCleanup(void) {
> > > +     if (!xmlDictInitialized)
> > > +         return;
> > > +
> > > +     xmlFreeRMutex(xmlDictMutex);
> > > + }
> >
> > I forgot to add one line, this will probably want
> > xmlDictInitialized = 0;
> > at the end of the function.
>
>
>   Seems your first mail didn't make it to the list. Please subscribe.
> I don't know what you're making reference to !

Subscribed now - here's the first mail I sent:

Hi there,
I was debugging an issue today where, under load, I would get a crash 
(well, abort() due to double-free) like
#5  0x081b9ded in xmlDictFree (dict=0x8762ec0) at dict.c:472
#6  0x08192a0c in xmlFreeDoc (cur=0x8766158) at tree.c:1157
#7  0x081c9bf5 in xsltFreeStylesheet (sheet=0x8766268) at xslt.c:495

The program's usage of libxslt involves (potentially) many simultaneous 
transformations using different XSLT contexts (xsltNewTransformContext). 
Looking at the code, I felt that the most likely cause of this was the 
sub-dictionaries used by XSLT (and yes it was).

I don't know if you think this is a situation that is worth a patch 
against; I can either place mutexes internally in libxml2, or around our 
calls to create/destroy XSLT contexts. I don't really mind either way, 
but this might help other people if they try something similar.

Anyway, here is a suggested patch. I know that as written, xmlDictInit() 
is not thread-safe - but the worst that can happen is a leaked mutex, 
and I wanted to mirror the init/cleanup routines in the catalog code.

Thanks,
Gary.

*** tmp/libxml2-2.6.17/dict.c   Tue Jan  4 14:49:47 2005
--- libxml2/dict.c      Thu Jan 20 20:45:04 2005
***************
*** 70,75 ****
--- 70,115 ----
  };

  /*
+  * A mutex for modifying the reference counter for shared
+  * dictionaries.
+  */
+ static xmlRMutexPtr xmlDictMutex = NULL;
+
+ /*
+  * Whether the dictionary mutex was initialized.
+  */
+ static int xmlDictInitialized = 0;
+
+ /**
+  * xmlInitializeCatalog:
+  *
+  * Do the dictionary mutex initialization.
+  * this function is not thread safe, initialization should
+  * preferably be done once at startup
+  */
+ static void xmlInitializeDict() {
+     if (xmlDictInitialized)
+         return;
+
+     xmlDictMutex = xmlNewRMutex();
+
+     xmlDictInitialized = 1;
+ }
+
+ /**
+  * xmlCatalogCleanup:
+  *
+  * Free the dictionary mutex.
+  */
+ void
+ xmlDictCleanup(void) {
+     if (!xmlDictInitialized)
+         return;
+
+     xmlFreeRMutex(xmlDictMutex);
+ }
+
+ /*
   * xmlDictAddString:
   * @dict: the dictionnary
   * @name: the name of the userdata
***************
*** 328,335 ****
--- 368,380 ----
   */
  int
  xmlDictReference(xmlDictPtr dict) {
+     if (!xmlDictInitialized)
+         xmlInitializeDict();
+
      if (dict == NULL) return -1;
+     xmlRMutexLock(xmlDictMutex);
      dict->ref_counter++;
+     xmlRMutexUnlock(xmlDictMutex);
      return(0);
  }

***************
*** 445,453 ****
      if (dict == NULL)
        return;

      /* decrement the counter, it may be shared by a parser and docs */
      dict->ref_counter--;
!     if (dict->ref_counter > 0) return;

      if (dict->subdict != NULL) {
          xmlDictFree(dict->subdict);
--- 490,507 ----
      if (dict == NULL)
        return;

+     if (!xmlDictInitialized)
+         xmlInitializeDict();
+
      /* decrement the counter, it may be shared by a parser and docs */
+     xmlRMutexLock(xmlDictMutex);
      dict->ref_counter--;
!     if (dict->ref_counter > 0) {
!         xmlRMutexUnlock(xmlDictMutex);
!         return;
!     }
!
!     xmlRMutexUnlock(xmlDictMutex);

      if (dict->subdict != NULL) {
          xmlDictFree(dict->subdict);




*** tmp/libxml2-2.6.17/parser.c Sun Jan 16 18:43:47 2005
--- libxml2/parser.c    Thu Jan 20 20:13:34 2005
***************
*** 12158,12163 ****
--- 12158,12164 ----
  #ifdef LIBXML_CATALOG_ENABLED
      xmlCatalogCleanup();
  #endif
+     xmlDictCleanup();
      xmlCleanupInputCallbacks();
  #ifdef LIBXML_OUTPUT_ENABLED
      xmlCleanupOutputCallbacks();





*** tmp/libxml2-2.6.17/include/libxml/dict.h    Tue Jan  4 14:49:49 2005
--- libxml2/include/libxml/dict.h       Thu Jan 20 20:35:09 2005
***************
*** 56,61 ****
--- 56,68 ----
                                         const xmlChar *str);
  XMLPUBFUN int XMLCALL
                        xmlDictSize     (xmlDictPtr dict);
+
+ /*
+  * Cleanup function
+  */
+ XMLPUBFUN void XMLCALL
+                       xmlDictCleanup  (void);
+
  #ifdef __cplusplus
  }
  #endif