From 5a1a73b4326d97789b5640225be0e25cb8dd8de7 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 30 Sep 2008 14:20:01 +0200 Subject: 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 --- runtime/net.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) (limited to 'runtime/net.c') 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) { -- cgit v1.2.3 From e30fe2842e3ce9f06098c18f0f962596059684ca Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 30 Sep 2008 15:57:10 +0200 Subject: prevent getnameinfo() from being cancelled ... but removed the mutex, as the problem seems to be in cancel processing, so the mutex is no longer necessary --- runtime/net.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'runtime/net.c') diff --git a/runtime/net.c b/runtime/net.c index 892edf4a..31b02f1a 100644 --- a/runtime/net.c +++ b/runtime/net.c @@ -685,11 +685,14 @@ mygetnameinfo(const struct sockaddr *sa, socklen_t salen, char *serv, size_t servlen, int flags) { static pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER; + int iCancelStateSave; int i; - pthread_mutex_lock(&mut); + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave); + //pthread_mutex_lock(&mut); i = getnameinfo(sa, salen, host, hostlen, serv, servlen, flags); - pthread_mutex_unlock(&mut); + //pthread_mutex_unlock(&mut); + pthread_setcancelstate(iCancelStateSave, NULL); return i; } -- cgit v1.2.3 From 86f76c1299b7827549a0cd754960347af627e18d Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 2 Oct 2008 10:44:44 +0200 Subject: removed no longer needed mutex calls problem source is that getnameinfo() is not cancel-safe, not that it is not thread-safe. It is now guarded against cancellation. --- runtime/net.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'runtime/net.c') diff --git a/runtime/net.c b/runtime/net.c index 31b02f1a..85d6813b 100644 --- a/runtime/net.c +++ b/runtime/net.c @@ -684,14 +684,11 @@ 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 iCancelStateSave; int i; pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave); - //pthread_mutex_lock(&mut); i = getnameinfo(sa, salen, host, hostlen, serv, servlen, flags); - //pthread_mutex_unlock(&mut); pthread_setcancelstate(iCancelStateSave, NULL); return i; } -- cgit v1.2.3 From 1908bae50491624d270c73609ea45634c15246fd Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 2 Oct 2008 11:01:08 +0200 Subject: fixed git merge bug (duplicate code could not be detected) --- runtime/net.c | 19 ------------------- 1 file changed, 19 deletions(-) (limited to 'runtime/net.c') diff --git a/runtime/net.c b/runtime/net.c index 38b41389..44c9008a 100644 --- a/runtime/net.c +++ b/runtime/net.c @@ -695,25 +695,6 @@ 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) -{ - int iCancelStateSave; - int i; - - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave); - i = getnameinfo(sa, salen, host, hostlen, serv, servlen, flags); - pthread_setcancelstate(iCancelStateSave, NULL); - 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 -- cgit v1.2.3