diff options
| -rw-r--r-- | ChangeLog | 10 | ||||
| -rw-r--r-- | configure.ac | 2 | ||||
| -rw-r--r-- | plugins/imudp/imudp.c | 8 | ||||
| -rw-r--r-- | threads.c | 80 | ||||
| -rw-r--r-- | threads.h | 3 |
5 files changed, 72 insertions, 31 deletions
@@ -8,13 +8,9 @@ Version 5.3.2 [DEVEL] (rgerhards), 2009-10-?? rsyslogd termination (canceling threads my result in not properly freed resouces and potential later hangs, even though we perform proper cancel handling in our code). This is part of an effort to - reduce thread cnacellation as much as possible in rsyslog. - NOTE: some comments indicated that there were problems with some code - that has been re-activated. Testing did not show any issues. My current - assumption is that these issues were related to some other code that - has been removed/changed during the previous restructuring events. - In any case, if there is a shutdown issue, one should carefully look - at this change here! + reduce thread cancellation as much as possible in rsyslog. + NOTE: the code previously written code for this functionality had a + subtle race condition. The new code solves that. - enhanced immark to support non-cancel input module termination - improved imudp so that epoll can be used in more environments, fixed potential compile time problem if EPOLL_CLOEXEC is not available. diff --git a/configure.ac b/configure.ac index 8c782762..376855b3 100644 --- a/configure.ac +++ b/configure.ac @@ -15,7 +15,7 @@ AC_GNU_SOURCE # check for Java compiler AC_CHECK_PROG(HAVE_JAVAC, [javac], [yes]) -if test x"$HAVE_JAVAC" = x"yes"; then +if test x"$HAVE_JAVAC" = x""; then AC_MSG_WARN([no javac found, disabling features depending on it]) fi diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index 3159de4f..f8227fa7 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -295,7 +295,7 @@ finalize_it: */ #if defined(HAVE_EPOLL_CREATE1) || defined(HAVE_EPOLL_CREATE) #define NUM_EPOLL_EVENTS 10 -rsRetVal rcvMainLoop() +rsRetVal rcvMainLoop(thrdInfo_t *pThrd) { DEFiRet; int nfds; @@ -350,7 +350,7 @@ rsRetVal rcvMainLoop() nfds = epoll_wait(efd, currEvt, NUM_EPOLL_EVENTS, -1); DBGPRINTF("imudp: epoll_wait() returned with %d fds\n", nfds); - if(glbl.GetGlobalInputTermState() == 1) + if(pThrd->bShallStop == TRUE) break; /* terminate input! */ for(i = 0 ; i < nfds ; ++i) { @@ -367,7 +367,7 @@ finalize_it: } #else /* #if HAVE_EPOLL_CREATE1 */ /* this is the code for the select() interface */ -rsRetVal rcvMainLoop() +rsRetVal rcvMainLoop(thrdInfo_t *pThrd) { DEFiRet; int maxfds; @@ -443,7 +443,7 @@ CODESTARTrunInput * signalled to do so. This, however, is handled by the framework, * right into the sleep below. */ - iRet = rcvMainLoop(); + iRet = rcvMainLoop(pThrd); ENDrunInput @@ -5,7 +5,7 @@ * * File begun on 2007-12-14 by RGerhards * - * Copyright 2007 Rainer Gerhards and Adiscon GmbH. + * Copyright 2007, 2009 Rainer Gerhards and Adiscon GmbH. * * This file is part of rsyslog. * @@ -29,6 +29,7 @@ #include <stdlib.h> #include <string.h> #include <signal.h> +#include <errno.h> #include <pthread.h> #include <assert.h> @@ -36,6 +37,7 @@ #include "dirty.h" #include "linkedlist.h" #include "threads.h" +#include "srUtils.h" /* linked list of currently-known threads */ static linkedList_t llThrds; @@ -44,7 +46,8 @@ static linkedList_t llThrds; /* Construct a new thread object */ -static rsRetVal thrdConstruct(thrdInfo_t **ppThis) +static rsRetVal +thrdConstruct(thrdInfo_t **ppThis) { DEFiRet; thrdInfo_t *pThis; @@ -52,13 +55,8 @@ static rsRetVal thrdConstruct(thrdInfo_t **ppThis) assert(ppThis != NULL); CHKmalloc(pThis = calloc(1, sizeof(thrdInfo_t))); - - /* OK, we got the element, now initialize members that should - * not be zero-filled. - */ - pThis->mutTermOK = (pthread_mutex_t *) malloc (sizeof (pthread_mutex_t)); - pthread_mutex_init (pThis->mutTermOK, NULL); - + pthread_mutex_init(&pThis->mutThrd, NULL); + pthread_cond_init(&pThis->condThrdTerm, NULL); *ppThis = pThis; finalize_it: @@ -78,13 +76,54 @@ static rsRetVal thrdDestruct(thrdInfo_t *pThis) if(pThis->bIsActive == 1) { thrdTerminate(pThis); } - free(pThis->mutTermOK); + pthread_mutex_destroy(&pThis->mutThrd); + pthread_cond_destroy(&pThis->condThrdTerm); free(pThis); RETiRet; } +/* terminate a thread via the non-cancel interface + * This is a separate function as it involves a bit more of code. + * rgerhads, 2009-10-15 + */ +static inline rsRetVal +thrdTerminateNonCancel(thrdInfo_t *pThis) +{ + struct timespec tTimeout; + int ret; + DEFiRet; + assert(pThis != NULL); + + DBGPRINTF("request term via SIGTTIN for input thread 0x%x\n", (unsigned) pThis->thrdID); + pThis->bShallStop = TRUE; + do { + d_pthread_mutex_lock(&pThis->mutThrd); + pthread_kill(pThis->thrdID, SIGTTIN); + timeoutComp(&tTimeout, 10); /* a fixed 10ms timeout, do after lock (may take long!) */ + ret = d_pthread_cond_timedwait(&pThis->condThrdTerm, &pThis->mutThrd, &tTimeout); + d_pthread_mutex_unlock(&pThis->mutThrd); + if(Debug) { + if(ret == ETIMEDOUT) { + dbgprintf("input thread term: had a timeout waiting on thread termination\n"); + } else if(ret == 0) { + dbgprintf("input thread term: thread returned normally and is terminated\n"); + } else { + char errStr[1024]; + int err = errno; + rs_strerror_r(err, errStr, sizeof(errStr)); + dbgprintf("input thread term: cond_wait returned with error %d: %s\n", + err, errStr); + } + } + } while(pThis->bIsActive); + DBGPRINTF("non-cancel input thread termination succeeded for thread 0x%x\n", (unsigned) pThis->thrdID); + + RETiRet; +} + + /* terminate a thread gracefully. */ rsRetVal thrdTerminate(thrdInfo_t *pThis) @@ -95,13 +134,11 @@ rsRetVal thrdTerminate(thrdInfo_t *pThis) if(pThis->bNeedsCancel) { DBGPRINTF("request term via canceling for input thread 0x%x\n", (unsigned) pThis->thrdID); pthread_cancel(pThis->thrdID); + pThis->bIsActive = 0; } else { - - DBGPRINTF("request term via SIGTTIN for input thread 0x%x\n", (unsigned) pThis->thrdID); - pthread_kill(pThis->thrdID, SIGTTIN); + thrdTerminateNonCancel(pThis); } pthread_join(pThis->thrdID, NULL); /* wait for input thread to complete */ - pThis->bIsActive = 0; /* call cleanup function, if any */ if(pThis->pAfterRun != NULL) @@ -152,6 +189,16 @@ static void* thrdStarter(void *arg) iRet = pThis->pUsrThrdMain(pThis); dbgprintf("thrdStarter: usrThrdMain 0x%lx returned with iRet %d, exiting now.\n", (unsigned long) pThis->thrdID, iRet); + + /* signal master control that we exit (we do the mutex lock mostly to + * keep the thread debugger happer, it would not really be necessary with + * the logic we employ...) + */ + pThis->bIsActive = 0; + d_pthread_mutex_lock(&pThis->mutThrd); + pthread_cond_signal(&pThis->condThrdTerm); + d_pthread_mutex_unlock(&pThis->mutThrd); + ENDfunc pthread_exit(0); } @@ -198,9 +245,7 @@ rsRetVal thrdInit(void) rsRetVal thrdExit(void) { DEFiRet; - iRet = llDestroy(&llThrds); - RETiRet; } @@ -229,6 +274,5 @@ thrdSleep(thrdInfo_t *pThis, int iSeconds, int iuSeconds) } -/* - * vi:set ai: +/* vi:set ai: */ @@ -25,7 +25,8 @@ /* the thread object */ struct thrdInfo { - pthread_mutex_t *mutTermOK; /* Is it ok to terminate that thread now? */ + pthread_mutex_t mutThrd;/* mutex for handling long-running operations and shutdown */ + pthread_cond_t condThrdTerm;/* condition: thread terminates (used just for shutdown loop) */ int bIsActive; /* Is thread running? */ int bShallStop; /* set to 1 if the thread should be stopped ? */ rsRetVal (*pUsrThrdMain)(struct thrdInfo*); /* user thread main to be called in new thread */ |