bugfix: potential hang due to mutex deadlock

closes: http://bugzilla.adiscon.com/show_bug.cgi?id=316
Thanks to Andreas Piesk for reporting&analyzing this bug as well as
providing patches and other help in resolving it.

3 files changed, 10 insertions, 29 deletions

diff --git a/runtime/modules.c b/runtime/modules.c
index 4541bddf..c2054e40 100644
--- a/runtime/modules.c
+++ b/runtime/modules.c
@@ -66,14 +66,6 @@ DEFobjCurrIf(errmsg)
 DEFobjCurrIf(parser)
 DEFobjCurrIf(strgen)
 
-/* we must ensure that only one thread at one time tries to load or unload
- * modules, otherwise we may see race conditions. This first came up with
- * imdiag/imtcp, which both use the same stream drivers. Below is the mutex
- * for that handling.
- * rgerhards, 2009-05-25
- */
-static pthread_mutex_t mutLoadUnload;
-
 static modInfo_t *pLoadedModules = NULL;	/* list of currently-loaded modules */
 static modInfo_t *pLoadedModulesLast = NULL;	/* tail-pointer */
 
@@ -667,7 +659,7 @@ modUnlinkAndDestroy(modInfo_t **ppThis)
 	pThis = *ppThis;
 	assert(pThis != NULL);
 
-	pthread_mutex_lock(&mutLoadUnload);
+	pthread_mutex_lock(&mutObjGlobalOp);
 
 	/* first check if we are permitted to unload */
 	if(pThis->eType == eMOD_LIB) {
@@ -703,7 +695,7 @@ modUnlinkAndDestroy(modInfo_t **ppThis)
 	moduleDestruct(pThis);
 
 finalize_it:
-	pthread_mutex_unlock(&mutLoadUnload);
+	pthread_mutex_unlock(&mutObjGlobalOp);
 	RETiRet;
 }
 
@@ -779,7 +771,7 @@ Load(uchar *pModName)
 	assert(pModName != NULL);
 	dbgprintf("Requested to load module '%s'\n", pModName);
 
-	pthread_mutex_lock(&mutLoadUnload);
+	pthread_mutex_lock(&mutObjGlobalOp);
 
 	iModNameLen = strlen((char *) pModName);
 	if(iModNameLen > 3 && !strcmp((char *) pModName + iModNameLen - 3, ".so")) {
@@ -903,7 +895,7 @@ Load(uchar *pModName)
 	}
 
 finalize_it:
-	pthread_mutex_unlock(&mutLoadUnload);
+	pthread_mutex_unlock(&mutObjGlobalOp);
 	RETiRet;
 }
 
@@ -1000,16 +992,6 @@ CODESTARTObjClassExit(module)
 	/* release objects we no longer need */
 	objRelease(errmsg, CORE_COMPONENT);
 	objRelease(parser, CORE_COMPONENT);
-	/* We have a problem in our reference counting, which leads to this function
-	 * being called too early. This usually is no problem, but if we destroy
-	 * the mutex object, we get into trouble. So rather than finding the root cause,
-	 * we do not release the mutex right now and have a very, very slight leak.
-	 * We know that otherwise no bad effects happen, so this acceptable for the 
-	 * time being. -- rgerhards, 2009-05-25
-	 *
-	 * TODO: add again: pthread_mutex_destroy(&mutLoadUnload);
-	 */
-
 #	ifdef DEBUG
 	modUsrPrintAll(); /* debug aid - TODO: integrate with debug.c, at least the settings! */
 #	endif
@@ -1051,7 +1033,6 @@ ENDobjQueryInterface(module)
  */
 BEGINAbstractObjClassInit(module, 1, OBJ_IS_CORE_MODULE) /* class, version - CHANGE class also in END MACRO! */
 	uchar *pModPath;
-	pthread_mutexattr_t mutAttr;
 
 	/* use any module load path specified in the environment */
 	if((pModPath = (uchar*) getenv("RSYSLOG_MODDIR")) != NULL) {
@@ -1069,10 +1050,6 @@ BEGINAbstractObjClassInit(module, 1, OBJ_IS_CORE_MODULE) /* class, version - CHA
 		SetModDir(glblModPath);
 	}
 
-	pthread_mutexattr_init(&mutAttr);
-	pthread_mutexattr_settype(&mutAttr, PTHREAD_MUTEX_RECURSIVE);
-	pthread_mutex_init(&mutLoadUnload, &mutAttr);
-
 	/* request objects we use */
 	CHKiRet(objUse(errmsg, CORE_COMPONENT));
 ENDObjClassInit(module)
diff --git a/runtime/obj.c b/runtime/obj.c
index 29ca8117..f9afecd8 100644
--- a/runtime/obj.c
+++ b/runtime/obj.c
@@ -97,7 +97,7 @@ DEFobjCurrIf(module)
 DEFobjCurrIf(errmsg)
 DEFobjCurrIf(strm)
 static objInfo_t *arrObjInfo[OBJ_NUM_IDS]; /* array with object information pointers */
-static pthread_mutex_t mutObjGlobalOp;	/* mutex to guard global operations of the object system */
+pthread_mutex_t mutObjGlobalOp;	/* mutex to guard global operations of the object system */
 
 
 /* cookies for serialized lines */
@@ -1319,7 +1319,7 @@ objClassInit(modInfo_t *pModInfo)
 	}
 
 	/* the mutex must be recursive, because objects may call into other
-	 * object identifieres recursively.
+	 * object identifiers recursively.
 	 */
 	pthread_mutexattr_init(&mutAttr);
 	pthread_mutexattr_settype(&mutAttr, PTHREAD_MUTEX_RECURSIVE);
diff --git a/runtime/obj.h b/runtime/obj.h
index be97f20e..32f7ef09 100644
--- a/runtime/obj.h
+++ b/runtime/obj.h
@@ -122,4 +122,8 @@ rsRetVal objGetObjInterface(obj_if_t *pIf);
 PROTOTYPEObjClassInit(obj);
 PROTOTYPEObjClassExit(obj);
 
+
+/* the following definition is only for "friends" */
+extern pthread_mutex_t mutObjGlobalOp;	/* mutex to guard global operations of the object system */
+
 #endif /* #ifndef OBJ_H_INCLUDED */