improved threading

- changed sequence when awakening thread
- removed no longer needed condition variable
- EXPERIMENTALLY added mutex guarding to hostname lookups
  this is to be removed if it does not have any verifyable
  useful effect

5 files changed, 35 insertions, 13 deletions

diff --git a/runtime/net.c b/runtime/net.c
index f5b8f46a..892edf4a 100644
--- a/runtime/net.c
+++ b/runtime/net.c
@@ -674,6 +674,25 @@ finalize_it:
 }
 
 
+
+/* This is a synchronized getnameinfo() version, because we learned
+ * (via drd/valgrind) that getnameinfo() seems to have some multi-threading
+ * issues. -- rgerhards, 2008-09-30
+ */
+static int
+mygetnameinfo(const struct sockaddr *sa, socklen_t salen,
+                       char *host, size_t hostlen,
+                       char *serv, size_t servlen, int flags)
+{
+	static pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER;
+	int i;
+
+	pthread_mutex_lock(&mut);
+	i = getnameinfo(sa, salen, host, hostlen, serv, servlen, flags);
+	pthread_mutex_unlock(&mut);
+	return i;
+}
+
 /* Print an allowed sender list. The caller must tell us which one.
  * iListToPrint = 1 means UDP, 2 means TCP
  * rgerhards, 2005-09-27
@@ -708,7 +727,7 @@ void PrintAllowedSenders(int iListToPrint)
 			if (F_ISSET(pSender->allowedSender.flags, ADDR_NAME))
 				dbgprintf ("\t%s\n", pSender->allowedSender.addr.HostWildcard);
 			else {
-				if(getnameinfo (pSender->allowedSender.addr.NetAddr,
+				if(mygetnameinfo (pSender->allowedSender.addr.NetAddr,
 						     SALEN(pSender->allowedSender.addr.NetAddr),
 						     (char*)szIP, 64, NULL, 0, NI_NUMERICHOST) == 0) {
 					dbgprintf ("\t%s/%u\n", szIP, pSender->SignificantBits);
@@ -956,7 +975,6 @@ should_use_so_bsdcompat(void)
 #define SO_BSDCOMPAT 0
 #endif
 
-
 /* get the hostname of the message source. This was originally in cvthname()
  * but has been moved out of it because of clarity and fuctional separation.
  * It must be provided by the socket we received the message on as well as
@@ -982,7 +1000,7 @@ gethname(struct sockaddr_storage *f, uchar *pszHostFQDN, uchar *ip)
 	assert(f != NULL);
 	assert(pszHostFQDN != NULL);
 
-        error = getnameinfo((struct sockaddr *)f, SALEN((struct sockaddr *)f),
+        error = mygetnameinfo((struct sockaddr *)f, SALEN((struct sockaddr *)f),
 			    (char*) ip, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
 
         if (error) {
@@ -997,7 +1015,7 @@ gethname(struct sockaddr_storage *f, uchar *pszHostFQDN, uchar *ip)
 		sigaddset(&nmask, SIGHUP);
 		pthread_sigmask(SIG_BLOCK, &nmask, &omask);
 
-		error = getnameinfo((struct sockaddr *)f, SALEN((struct sockaddr *) f),
+		error = mygetnameinfo((struct sockaddr *)f, SALEN((struct sockaddr *) f),
 				    (char*)pszHostFQDN, NI_MAXHOST, NULL, 0, NI_NAMEREQD);
 		
 		if (error == 0) {
diff --git a/runtime/queue.c b/runtime/queue.c
index 3fae4aa7..76c2f10f 100644
--- a/runtime/queue.c
+++ b/runtime/queue.c
@@ -1430,8 +1430,13 @@ queueDequeueConsumable(queue_t *pThis, wti_t *pWti, int iCancelStateSave)
 		pthread_cond_broadcast(&pThis->belowLightDlyWtrMrk);
 	}
 
-	d_pthread_mutex_unlock(pThis->mut);
+	/* rgerhards, 2008-09-30: I reversed the order of cond_signal und mutex_unlock
+	 * as of the pthreads recommendation on predictable scheduling behaviour. I don't see
+	 * any problems caused by this, but I add this comment in case some will be seen
+	 * in the next time.
+	 */
 	pthread_cond_signal(&pThis->notFull);
+	d_pthread_mutex_unlock(pThis->mut);
 	pthread_setcancelstate(iCancelStateSave, NULL);
 	/* WE ARE NO LONGER PROTECTED BY THE MUTEX */
 
diff --git a/runtime/wti.c b/runtime/wti.c
index 13554232..365b25d5 100644
--- a/runtime/wti.c
+++ b/runtime/wti.c
@@ -113,6 +113,9 @@ wtiSetState(wti_t *pThis, qWrkCmd_t tCmd, int bActiveOnly, int bLockMutex)
 			  wtiGetDbgHdr(pThis), tCmd, pThis->tCurrCmd);
 	} else {
 		dbgprintf("%s: receiving command %d\n", wtiGetDbgHdr(pThis), tCmd);
+		/* we could replace this with a simple if, but we leave the switch in in case we need
+		 * to add something at a later stage. -- rgerhards, 2008-09-30
+		 */
 		switch(tCmd) {
 			case eWRKTHRD_TERMINATING:
 				/* TODO: re-enable meaningful debug msg! (via function callback?)
@@ -123,10 +126,8 @@ wtiSetState(wti_t *pThis, qWrkCmd_t tCmd, int bActiveOnly, int bLockMutex)
 				pthread_cond_signal(&pThis->condExitDone);
 				dbgprintf("%s: worker terminating\n", wtiGetDbgHdr(pThis));
 				break;
-			case eWRKTHRD_RUNNING:
-				pthread_cond_signal(&pThis->condInitDone);
-				break;
 			/* these cases just to satisfy the compiler, we do (yet) not act an them: */
+			case eWRKTHRD_RUNNING:
 			case eWRKTHRD_STOPPED:
 			case eWRKTHRD_RUN_CREATED:
 			case eWRKTHRD_RUN_INIT:
@@ -190,7 +191,6 @@ CODESTARTobjDestruct(wti)
 	d_pthread_mutex_unlock(&pThis->mut);
 
 	/* actual destruction */
-	pthread_cond_destroy(&pThis->condInitDone);
 	pthread_cond_destroy(&pThis->condExitDone);
 	pthread_mutex_destroy(&pThis->mut);
 
@@ -202,7 +202,6 @@ ENDobjDestruct(wti)
 /* Standard-Constructor for the wti object
  */
 BEGINobjConstruct(wti) /* be sure to specify the object type also in END macro! */
-	pthread_cond_init(&pThis->condInitDone, NULL);
 	pthread_cond_init(&pThis->condExitDone, NULL);
 	pthread_mutex_init(&pThis->mut, NULL);
 ENDobjConstruct(wti)
diff --git a/runtime/wti.h b/runtime/wti.h
index b3d92473..0cd6744e 100644
--- a/runtime/wti.h
+++ b/runtime/wti.h
@@ -35,7 +35,6 @@ typedef struct wti_s {
 	qWrkCmd_t tCurrCmd; /* current command to be carried out by worker */
 	obj_t *pUsrp;		/* pointer to an object meaningful for current user pointer (e.g. queue pUsr data elemt) */
 	wtp_t *pWtp; /* my worker thread pool (important if only the work thread instance is passed! */
-	pthread_cond_t condInitDone; /* signaled when the thread startup is done (once per thread existance) */
 	pthread_cond_t condExitDone; /* signaled when the thread exit is done (once per thread existance) */
 	pthread_mutex_t mut;
 	int bShutdownRqtd;	/* shutdown for this thread requested? 0 - no , 1 - yes */
diff --git a/runtime/wtp.c b/runtime/wtp.c
index 54f12b2b..734c8d57 100644
--- a/runtime/wtp.c
+++ b/runtime/wtp.c
@@ -304,11 +304,12 @@ wtpShutdownAll(wtp_t *pThis, wtpState_t tShutdownCmd, struct timespec *ptTimeout
 rsRetVal wtpSignalWrkrTermination(wtp_t *pThis)
 {
 	DEFiRet;
-	/* I leave the mutex code here out as it give as deadlocks. I think it is not really
+	/* I leave the mutex code here out as it gives us deadlocks. I think it is not really
 	 * needed and we are on the safe side. I leave this comment in if practice proves us
-	 * wrong. The whole thing should be removed after half a your or year if we see there
+	 * wrong. The whole thing should be removed after half a year or year if we see there
 	 * actually is no issue (or revisit it from a theoretical POV).
 	 * rgerhards, 2008-01-28
+	 * revisited 2008-09-30, still a bit unclear, leave in
 	 */
 	/*TODO: mutex or not mutex, that's the question ;)DEFVARS_mutexProtection;*/