trying to cleanup the not thread safe parts of the library. Daniel
* catalog.c xpath.c: trying to cleanup the not thread safe
parts of the library.
Daniel
diff --git a/catalog.c b/catalog.c
index ef8a0c7..fca4022 100644
--- a/catalog.c
+++ b/catalog.c
@@ -37,6 +37,7 @@
#include <libxml/parserInternals.h>
#include <libxml/catalog.h>
#include <libxml/xmlerror.h>
+#include <libxml/threads.h>
#define MAX_DELEGATE 50
@@ -139,6 +140,10 @@
* *
************************************************************************/
+/*
+ * Those are preferences
+ */
+static int xmlDebugCatalogs = 0; /* used for debugging */
static xmlCatalogAllow xmlCatalogDefaultAllow = XML_CATA_ALLOW_ALL;
static xmlCatalogPrefer xmlCatalogDefaultPrefer = XML_CATA_PREFER_PUBLIC;
@@ -154,13 +159,19 @@
static xmlCatalogPtr xmlDefaultCatalog = NULL;
/*
+ * A mutex for modifying the shared global catalog(s)
+ * xmlDefaultCatalog tree.
+ * It also protects xmlCatalogXMLFiles
+ * The core of this readers/writer scheme is in xmlFetchXMLCatalogFile()
+ */
+static xmlRMutexPtr xmlCatalogMutex = NULL;
+
+/*
* Whether the catalog support was initialized.
*/
static int xmlCatalogInitialized = 0;
-static int xmlDebugCatalogs = 0; /* used for debugging */
-
/************************************************************************
* *
* Allocation and Freeing *
@@ -1207,8 +1218,6 @@
* @catal: an existing but incomplete catalog entry
*
* Fetch and parse the subcatalog referenced by an entry
- * It tries to be thread safe but by lack of an atomic test and
- * set there is a risk of loosing memory.
*
* Returns 0 in case of success, -1 otherwise
*/
@@ -1223,12 +1232,25 @@
if (catal->children != NULL)
return(-1);
+ /*
+ * lock the whole catalog for modification
+ */
+ xmlRMutexLock(xmlCatalogMutex);
+ if (catal->children != NULL) {
+ /* Okay someone else did it in the meantime */
+ xmlRMutexUnlock(xmlCatalogMutex);
+ return(0);
+
+
+ }
+
if (xmlCatalogXMLFiles != NULL)
children = (xmlCatalogEntryPtr)
xmlHashLookup(xmlCatalogXMLFiles, catal->value);
if (children != NULL) {
catal->children = children;
catal->dealloc = 0;
+ xmlRMutexUnlock(xmlCatalogMutex);
return(0);
}
@@ -1240,6 +1262,7 @@
doc = xmlParseXMLCatalogFile(catal->prefer, catal->value);
if (doc == NULL) {
catal->type = XML_CATA_BROKEN_CATALOG;
+ xmlRMutexUnlock(xmlCatalogMutex);
return(-1);
}
if ((catal->type == XML_CATA_CATALOG) &&
@@ -1251,24 +1274,15 @@
children = doc;
}
- /*
- * Where a real test and set would be needed !
- */
- if (catal->children == NULL) {
- catal->children = children;
- catal->dealloc = 1;
- if (xmlCatalogXMLFiles == NULL)
- xmlCatalogXMLFiles = xmlHashCreate(10);
- if (xmlCatalogXMLFiles != NULL) {
- if (children != NULL)
- xmlHashAddEntry(xmlCatalogXMLFiles, catal->value, children);
- }
- } else {
- /*
- * Another thread filled it before us
- */
- xmlFreeCatalogEntryList(children);
+ catal->children = children;
+ catal->dealloc = 1;
+ if (xmlCatalogXMLFiles == NULL)
+ xmlCatalogXMLFiles = xmlHashCreate(10);
+ if (xmlCatalogXMLFiles != NULL) {
+ if (children != NULL)
+ xmlHashAddEntry(xmlCatalogXMLFiles, catal->value, children);
}
+ xmlRMutexUnlock(xmlCatalogMutex);
return(0);
}
@@ -2717,19 +2731,42 @@
************************************************************************/
/**
+ * xmlInitializeCatalogData:
+ *
+ * Do the catalog initialization only of global data, doesn't try to load
+ * any catalog actually.
+ * this function is not thread safe, catalog initialization should
+ * preferably be done once at startup
+ */
+static void
+xmlInitializeCatalogData(void) {
+ if (xmlCatalogInitialized != 0)
+ return;
+
+ if (getenv("XML_DEBUG_CATALOG"))
+ xmlDebugCatalogs = 1;
+ xmlCatalogMutex = xmlNewRMutex();
+
+ xmlCatalogInitialized = 1;
+}
+/**
* xmlInitializeCatalog:
*
* Do the catalog initialization.
- * TODO: this function is not thread safe, catalog initialization should
- * preferably be done once at startup
+ * this function is not thread safe, catalog initialization should
+ * preferably be done once at startup
*/
void
xmlInitializeCatalog(void) {
if (xmlCatalogInitialized != 0)
return;
+ xmlInitializeCatalogData();
+ xmlRMutexLock(xmlCatalogMutex);
+
if (getenv("XML_DEBUG_CATALOG"))
xmlDebugCatalogs = 1;
+
if (xmlDefaultCatalog == NULL) {
const char *catalogs;
xmlCatalogPtr catal;
@@ -2739,8 +2776,8 @@
catalogs = XML_XML_DEFAULT_CATALOG;
catal = xmlNewCatalog(XML_XML_CATALOG_TYPE, xmlCatalogDefaultPrefer);
- if (catal == NULL)
- return;
+ if (catal == NULL) {
+ }
catal->xml = xmlNewCatalogEntry(XML_CATA_CATALOG,
NULL, BAD_CAST catalogs, xmlCatalogDefaultPrefer);
@@ -2748,7 +2785,7 @@
xmlDefaultCatalog = catal;
}
- xmlCatalogInitialized = 1;
+ xmlRMutexUnlock(xmlCatalogMutex);
}
@@ -2758,8 +2795,8 @@
*
* Load the catalog and makes its definitions effective for the default
* external entity loader. It will recurse in SGML CATALOG entries.
- * TODO: this function is not thread safe, catalog initialization should
- * preferably be done once at startup
+ * this function is not thread safe, catalog initialization should
+ * preferably be done once at startup
*
* Returns 0 in case of success -1 in case of error
*/
@@ -2769,22 +2806,23 @@
int ret;
xmlCatalogPtr catal;
+ if (!xmlCatalogInitialized)
+ xmlInitializeCatalogData();
+
+ xmlRMutexLock(xmlCatalogMutex);
+
if (xmlDefaultCatalog == NULL) {
catal = xmlLoadACatalog(filename);
if (catal == NULL)
return(-1);
xmlDefaultCatalog = catal;
-
- /*
- * Need to be done after ...
- */
- if (!xmlCatalogInitialized)
- xmlInitializeCatalog();
+ xmlRMutexUnlock(xmlCatalogMutex);
return(0);
}
ret = xmlExpandCatalog(xmlDefaultCatalog, filename);
+ xmlRMutexUnlock(xmlCatalogMutex);
return(ret);
}
@@ -2794,8 +2832,8 @@
*
* Load the catalogs and makes their definitions effective for the default
* external entity loader.
- * TODO: this function is not thread safe, catalog initialization should
- * preferably be done once at startup
+ * this function is not thread safe, catalog initialization should
+ * preferably be done once at startup
*/
void
xmlLoadCatalogs(const char *pathss) {
@@ -2831,6 +2869,7 @@
*/
void
xmlCatalogCleanup(void) {
+ xmlRMutexLock(xmlCatalogMutex);
if (xmlDebugCatalogs)
xmlGenericError(xmlGenericErrorContext,
"Catalogs cleanup\n");
@@ -2842,6 +2881,8 @@
xmlDefaultCatalog = NULL;
xmlDebugCatalogs = 0;
xmlCatalogInitialized = 0;
+ xmlRMutexUnlock(xmlCatalogMutex);
+ xmlFreeRMutex(xmlCatalogMutex);
}
/**
@@ -2959,6 +3000,10 @@
xmlCatalogAdd(const xmlChar *type, const xmlChar *orig, const xmlChar *replace) {
int res = -1;
+ if (!xmlCatalogInitialized)
+ xmlInitializeCatalogData();
+
+ xmlRMutexLock(xmlCatalogMutex);
/*
* Specific case where one want to override the default catalog
* put in place by xmlInitializeCatalog();
@@ -2970,15 +3015,12 @@
xmlDefaultCatalog->xml = xmlNewCatalogEntry(XML_CATA_CATALOG, NULL,
orig, xmlCatalogDefaultPrefer);
- if (!xmlCatalogInitialized)
- xmlInitializeCatalog();
+ xmlRMutexUnlock(xmlCatalogMutex);
return(0);
}
- if (!xmlCatalogInitialized)
- xmlInitializeCatalog();
-
res = xmlACatalogAdd(xmlDefaultCatalog, type, orig, replace);
+ xmlRMutexUnlock(xmlCatalogMutex);
return(res);
}
@@ -2997,7 +3039,9 @@
if (!xmlCatalogInitialized)
xmlInitializeCatalog();
+ xmlRMutexLock(xmlCatalogMutex);
res = xmlACatalogRemove(xmlDefaultCatalog, value);
+ xmlRMutexUnlock(xmlCatalogMutex);
return(res);
}
@@ -3015,7 +3059,9 @@
if (!xmlCatalogInitialized)
xmlInitializeCatalog();
+ xmlRMutexLock(xmlCatalogMutex);
res = xmlConvertSGMLCatalog(xmlDefaultCatalog);
+ xmlRMutexUnlock(xmlCatalogMutex);
return(res);
}
@@ -3024,6 +3070,7 @@
* Public interface manipulating the common preferences *
* *
************************************************************************/
+
/**
* xmlCatalogGetDefaults:
*
@@ -3034,6 +3081,9 @@
*/
xmlCatalogAllow
xmlCatalogGetDefaults(void) {
+ if (!xmlCatalogInitialized)
+ xmlInitializeCatalog();
+
return(xmlCatalogDefaultAllow);
}
@@ -3047,6 +3097,7 @@
xmlCatalogSetDefaults(xmlCatalogAllow allow) {
if (!xmlCatalogInitialized)
xmlInitializeCatalog();
+
if (xmlDebugCatalogs) {
switch (allow) {
case XML_CATA_ALLOW_NONE:
@@ -3086,6 +3137,7 @@
if (!xmlCatalogInitialized)
xmlInitializeCatalog();
+
if (prefer == XML_CATA_PREFER_NONE)
return(ret);
@@ -3120,6 +3172,9 @@
xmlCatalogSetDebug(int level) {
int ret = xmlDebugCatalogs;
+ if (!xmlCatalogInitialized)
+ xmlInitializeCatalog();
+
if (level <= 0)
xmlDebugCatalogs = 0;
else
@@ -3143,6 +3198,9 @@
xmlCatalogFreeLocal(void *catalogs) {
xmlCatalogEntryPtr catal;
+ if (!xmlCatalogInitialized)
+ xmlInitializeCatalog();
+
catal = (xmlCatalogEntryPtr) catalogs;
if (catal != NULL)
xmlFreeCatalogEntryList(catal);
@@ -3164,6 +3222,7 @@
if (!xmlCatalogInitialized)
xmlInitializeCatalog();
+
if (URL == NULL)
return(catalogs);
@@ -3204,12 +3263,12 @@
xmlCatalogEntryPtr catal;
xmlChar *ret;
- if ((pubID == NULL) && (sysID == NULL))
- return(NULL);
-
if (!xmlCatalogInitialized)
xmlInitializeCatalog();
+ if ((pubID == NULL) && (sysID == NULL))
+ return(NULL);
+
if (xmlDebugCatalogs) {
if (pubID != NULL) {
xmlGenericError(xmlGenericErrorContext,
@@ -3245,12 +3304,12 @@
xmlCatalogEntryPtr catal;
xmlChar *ret;
- if (URI == NULL)
- return(NULL);
-
if (!xmlCatalogInitialized)
xmlInitializeCatalog();
+ if (URI == NULL)
+ return(NULL);
+
if (xmlDebugCatalogs)
xmlGenericError(xmlGenericErrorContext,
"Resolve URI %s\n", URI);
@@ -3284,6 +3343,9 @@
static xmlChar result[1000];
static int msg = 0;
+ if (!xmlCatalogInitialized)
+ xmlInitializeCatalog();
+
if (msg == 0) {
xmlGenericError(xmlGenericErrorContext,
"Use of deprecated xmlCatalogGetSystem() call\n");
@@ -3293,9 +3355,6 @@
if (sysID == NULL)
return(NULL);
- if (!xmlCatalogInitialized)
- xmlInitializeCatalog();
-
/*
* Check first the XML catalogs
*/
@@ -3328,6 +3387,9 @@
static xmlChar result[1000];
static int msg = 0;
+ if (!xmlCatalogInitialized)
+ xmlInitializeCatalog();
+
if (msg == 0) {
xmlGenericError(xmlGenericErrorContext,
"Use of deprecated xmlCatalogGetPublic() call\n");
@@ -3337,9 +3399,6 @@
if (pubID == NULL)
return(NULL);
- if (!xmlCatalogInitialized)
- xmlInitializeCatalog();
-
/*
* Check first the XML catalogs
*/