_mxml_init / _mxml_destructor causes SEGV in main app.

[michaelrsweet]

Version: 2.6
Original reporter:

We are developing an application plugin (a .so) on Linux that uses mxml. Unfortunately mxml causes the main application to SEGV. Having inspected the code thoroughly I have deduced why:
Main application (Rosegarden DAW) starts thread to scan for plugins
Rosegarden loads our plugin and invokes function to fetch descriptor plugin's descriptor.
libmxml.so is loaded automatically with plugin.so.
plugin.so loads some xml
_mxml_init is invoked. Thread specific key is created with _mxml_destructor attached.
plugin.so returns descriptor.
RG unloads plugin.so and libxml.so
RG thread ends. Thread cleanup calls _xml_destructor, which is no longer there - SEGV.

FIX - a fini routine is needed to call pthread_key_delete to remove the thread specific key.

[michaelrsweet]

"mxml-private.c.diff":

173a174,192
> #ifdef __GNUC__
> /*
>  * If we are unloaded before the thread terminates, make sure we free up
>  * the thread clean-up destructor.
>  *
>  */
> static void __attribute__ ((destructor))
> _mxml_fini(void)
> {
>   if(_mxml_key != -1)
>   {
>       void *g;
>       if((g = pthread_getspecific(_mxml_key)) != NULL)
>         free(g);
>       pthread_key_delete(_mxml_key);
>       _mxml_key = -1;
>   }
> }
> #endif

[michaelrsweet]

Original reporter:

I have attached a proposed patch to fix this bug.

[michaelrsweet]

Original reporter:

The patch is gnu (glibc) specific.

[michaelrsweet]

"mxml-private.c.diff":

173a174,192
> #ifdef __GNUC__
> /*
>  * If we are unloaded before the thread terminates, make sure we free up
>  * the thread clean-up destructor.
>  *
>  */
> static void __attribute__ ((destructor))
> _mxml_fini(void)
> {
>   if(_mxml_key != -1)
>   {
>       void *g;
>       if((g = pthread_getspecific(_mxml_key)) != NULL)
>         free(g);
>       pthread_key_delete(_mxml_key);
>       _mxml_key = -1;
>   }
> }
> #endif

[michaelrsweet]

Original reporter:

I have revised the patch to ensure that it will at least compile on non-gnu compilers that have pthread support. It does not fix the bug on these platforms; I only have access to gcc and msvc so I must leave that to others to implement - use of _fini should work on some, but not necessarily all.
If anyone has Solaris C I believe that the use of:
#pragma fini (_mxml_fini)
and #ifdef-ing out the attribute(destructor) may do the trick, but I have no way of checking.
alonbl - what compiler / platform are you targetting?

[michaelrsweet]

Original reporter: Michael Sweet

OK, so for starters it is basically never safe to call dlclose except for simple plugins with known dependencies. You always run the risk of leaving inconsistent state because of initializations that are done at load time. This isn't a problem specific to Mini-XML, and there is no portable way of dealing with it.

The GCC destructor attribute hack is very platform-specific. I will do some research to see if I can come up with an acceptable solution, but in the meantime I would file a bug with the program that is unloading plugins - that just isn't safe to do in the general case on pretty much any platform.

[michaelrsweet]

"str103.patch":

Index: mxml-private.c
===================================================================
--- mxml-private.c	(revision 406)
+++ mxml-private.c	(working copy)
@@ -3,7 +3,7 @@
  *
  * Private functions for Mini-XML, a small XML-like file parsing library.
  *
- * Copyright 2003-2007 by Michael Sweet.
+ * Copyright 2003-2010 by Michael Sweet.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -32,6 +32,33 @@
 
 
 /*
+ * Some crazy people think that unloading a shared object is a good or safe
+ * thing to do.  Unfortunately, most objects are simply *not* safe to unload
+ * and bad things *will* happen.
+ *
+ * The following mess of conditional code allows us to provide a destructor
+ * function in Mini-XML for our thread-global storage so that it can possibly
+ * be unloaded safely, although since there is no standard way to do so I
+ * can't even provide any guarantees that you can do it safely on all platforms.
+ *
+ * This code currently supports AIX, HP-UX, Linux, Mac OS X, Solaris, and
+ * Windows.  It might work on the BSDs and IRIX, but I haven't tested that.
+ */
+
+#if defined(__sun) || defined(_AIX)
+#  pragma fini(_mxml_fini)
+#  define _MXML_FINI _mxml_fini
+#elif defined(__hpux)
+#  pragma FINI _mxml_fini
+#  define _MXML_FINI _mxml_fini
+#elif defined(__GCC__) /* Linux and Mac OS X */
+#  define _MXML_FINI __attribute((destructor)) _mxml_fini
+#else
+#  define _MXML_FINI _fini
+#endif /* __sun */
+
+
+/*
  * 'mxml_error()' - Display an error message.
  */
 
@@ -136,6 +163,38 @@
 
 
 /*
+ * '_mxml_destructor()' - Free memory used for globals...
+ */
+
+static void
+_mxml_destructor(void *g)		/* I - Global data */
+{
+  free(g);
+}
+
+
+/*
+ * '_mxml_fini()' - Clean up when unloaded.
+ */
+
+static void
+_MXML_FINI(void)
+{
+  _mxml_global_t	*global;	/* Global data */
+
+
+  if (_mxml_key != -1)
+  {
+    if ((global = (_mxml_global_t *)pthread_getspecific(_mxml_key)) != NULL)
+      _mxml_destructor(global);
+
+    pthread_key_delete(_mxml_key);
+    _mxml_key = -1;
+  }
+}
+
+
+/*
  * '_mxml_global()' - Get global data.
  */
 
@@ -172,17 +231,6 @@
 }
 
 
-/*
- * '_mxml_destructor()' - Free memory used for globals...
- */
-
-static void
-_mxml_destructor(void *g)		/* I - Global data */
-{
-  free(g);
-}
-
-
 #elif defined(WIN32)			/**** WIN32 threading ****/
 #  include <windows.h>
 

[michaelrsweet]

Original reporter: Michael Sweet

Fixed in Subversion repository.