From ec56b763b83677d1e9cd02a7ae610caf62e902bb Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 8 Oct 2009 16:36:17 +0200 Subject: bugfix in debug system and more instrumentation to find an issue bugfix: debug string larger than 1K were improperly displayed. Max size is now 32K, and if a string is even longer it is meaningful truncated. --- ChangeLog | 4 ++++ configure.ac | 2 +- plugins/imudp/imudp.c | 3 ++- runtime/debug.c | 12 +++++++++++- runtime/parser.c | 16 ++++++++-------- template.c | 6 +++++- 6 files changed, 31 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8759c846..1c81f3aa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,8 @@ --------------------------------------------------------------------------- +Version 5.3.2 [DEVEL] (rgerhards), 2009-10-?? +- bugfix: debug string larger than 1K were improperly displayed. Max size + is now 32K, and if a string is even longer it is meaningful truncated. +--------------------------------------------------------------------------- Version 5.3.1 [DEVEL] (rgerhards), 2009-10-05 - added $AbortOnUncleanConfig directive - permits to prevent startup when there are problems with the configuration file. See it's doc for diff --git a/configure.ac b/configure.ac index 07fd0dac..e3f60b5c 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.61) -AC_INIT([rsyslog],[5.3.1],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[5.3.2],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index 735042a4..3fabf1a2 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -244,7 +244,8 @@ processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted, } } - DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%.80s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); + //DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%.80s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); + DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); if(*pbIsPermitted) { if((iTimeRequery == 0) || (iNbrTimeUsed++ % iTimeRequery) == 0) { diff --git a/runtime/debug.c b/runtime/debug.c index fb751efb..7c938008 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -960,7 +960,7 @@ void dbgprintf(char *fmt, ...) { va_list ap; - char pszWriteBuf[1024]; + char pszWriteBuf[32*1024]; size_t lenWriteBuf; if(!(Debug && debugging_on)) @@ -969,6 +969,16 @@ dbgprintf(char *fmt, ...) va_start(ap, fmt); lenWriteBuf = vsnprintf(pszWriteBuf, sizeof(pszWriteBuf), fmt, ap); va_end(ap); + + if(lenWriteBuf >= sizeof(pszWriteBuf)) { + /* if we need to truncate, do it in a somewhat useful way... */ + pszWriteBuf[sizeof(pszWriteBuf) - 5] = '!'; + pszWriteBuf[sizeof(pszWriteBuf) - 4] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 3] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 2] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 1] = '\n'; + lenWriteBuf = sizeof(pszWriteBuf); + } dbgprint(NULL, pszWriteBuf, lenWriteBuf); } diff --git a/runtime/parser.c b/runtime/parser.c index 466066e7..3c90c447 100644 --- a/runtime/parser.c +++ b/runtime/parser.c @@ -231,14 +231,14 @@ sanitizeMessage(msg_t *pMsg) * can not handle it! -- rgerhards, 2009-08-26 */ if(pszMsg[iSrc] == '\0' || bEscapeCCOnRcv) { - /* we are configured to escape control characters. Please note - * that this most probably break non-western character sets like - * Japanese, Korean or Chinese. rgerhards, 2007-07-17 - */ - pDst[iDst++] = cCCEscapeChar; - pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0300) >> 6); - pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0070) >> 3); - pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0007)); + /* we are configured to escape control characters. Please note + * that this most probably break non-western character sets like + * Japanese, Korean or Chinese. rgerhards, 2007-07-17 + */ + pDst[iDst++] = cCCEscapeChar; + pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0300) >> 6); + pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0070) >> 3); + pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0007)); } } else { pDst[iDst++] = pszMsg[iSrc]; diff --git a/template.c b/template.c index f3a8e057..1e0c9613 100644 --- a/template.c +++ b/template.c @@ -86,6 +86,7 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * unsigned short bMustBeFreed; uchar *pVal; size_t iLenVal; +int propid = -1; assert(pTpl != NULL); assert(pMsg != NULL); @@ -101,10 +102,12 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * iBuf = 0; while(pTpe != NULL) { if(pTpe->eEntryType == CONSTANT) { +propid = -1; pVal = (uchar*) pTpe->data.constant.pConstant; iLenVal = pTpe->data.constant.iLenConstant; bMustBeFreed = 0; } else if(pTpe->eEntryType == FIELD) { +propid = pTpe->data.field.propid; pVal = (uchar*) MsgGetProp(pMsg, pTpe, pTpe->data.field.propid, &iLenVal, &bMustBeFreed); /* we now need to check if we should use SQL option. In this case, * we must go over the generated string and escape '\'' characters. @@ -118,7 +121,8 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * doSQLEscape(&pVal, &iLenVal, &bMustBeFreed, 0); } /* got source, now copy over */ - if(iBuf + iLenVal + 1 >= *pLenBuf) /* we reserve one char for the final \0! */ +dbgprintf("copying prop id %3d (entry type %d) of length %d ('%s')\n", propid, pTpe->eEntryType, (int) iLenVal, pVal); + if(iBuf + iLenVal >= *pLenBuf) /* we reserve one char for the final \0! */ CHKiRet(ExtendBuf(ppBuf, pLenBuf, iBuf + iLenVal + 1)); if(iLenVal > 0) { /* may be zero depending on property */ -- cgit v1.2.3 From 3ed4b2cd3ebaf6f4c377ba2e03ef52c2e8a985b6 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 9 Oct 2009 14:48:25 +0200 Subject: bugfix: potential segfault on messages with empty MSG part. This was a recently introduced regression. --- ChangeLog | 2 ++ runtime/msg.c | 17 ++++++++++++++--- tools/syslogd.c | 6 ++---- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1c81f3aa..a9c1ad07 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ --------------------------------------------------------------------------- Version 5.3.2 [DEVEL] (rgerhards), 2009-10-?? +- bugfix: potential segfault on messages with empty MSG part. This was a + recently introduced regression. - bugfix: debug string larger than 1K were improperly displayed. Max size is now 32K, and if a string is even longer it is meaningful truncated. --------------------------------------------------------------------------- diff --git a/runtime/msg.c b/runtime/msg.c index 5a33837f..2c1af27e 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -1177,7 +1177,7 @@ uchar *getMSG(msg_t *pM) if(pM == NULL) ret = UCHAR_CONSTANT(""); else { - if(pM->offMSG == -1) + if(pM->iLenMSG == 0) ret = UCHAR_CONSTANT(""); else ret = pM->pszRawMsg + pM->offMSG; @@ -1953,12 +1953,22 @@ void MsgSetHOSTNAME(msg_t *pThis, uchar* pszHOSTNAME, int lenHOSTNAME) /* set the offset of the MSG part into the raw msg buffer + * Note that the offset may be higher than the length of the raw message + * (exactly by one). This can happen if we have a message that does not + * contain any MSG part. */ void MsgSetMSGoffs(msg_t *pMsg, short offs) { +BEGINfunc ISOBJ_TYPE_assert(pMsg, msg); - pMsg->iLenMSG = pMsg->iLenRawMsg - offs; pMsg->offMSG = offs; + if(offs > pMsg->iLenRawMsg) { + assert(offs - 1 == pMsg->iLenRawMsg); + pMsg->iLenMSG = 0; + } else { + pMsg->iLenMSG = pMsg->iLenRawMsg - offs; + } +ENDfunc } @@ -1992,7 +2002,8 @@ rsRetVal MsgReplaceMSG(msg_t *pThis, uchar* pszMSG, int lenMSG) pThis->pszRawMsg = bufNew; } - memcpy(pThis->pszRawMsg + pThis->offMSG, pszMSG, lenMSG); + if(lenMSG > 0) + memcpy(pThis->pszRawMsg + pThis->offMSG, pszMSG, lenMSG); pThis->pszRawMsg[lenNew] = '\0'; /* this also works with truncation! */ pThis->iLenRawMsg = lenNew; pThis->iLenMSG = lenMSG; diff --git a/tools/syslogd.c b/tools/syslogd.c index 0f4f8a23..3dc2d230 100644 --- a/tools/syslogd.c +++ b/tools/syslogd.c @@ -1205,8 +1205,6 @@ int parseLegacySyslogMsg(msg_t *pMsg, int flags) assert(pMsg != NULL); assert(pMsg->pszRawMsg != NULL); lenMsg = pMsg->iLenRawMsg - (pMsg->offAfterPRI + 1); -RUNLOG_VAR("%d", pMsg->offAfterPRI); -RUNLOG_VAR("%d", lenMsg); p2parse = pMsg->pszRawMsg + pMsg->offAfterPRI; /* point to start of text, after PRI */ /* Check to see if msg contains a timestamp. We start by assuming @@ -1262,16 +1260,16 @@ RUNLOG_VAR("%d", lenMsg); bTAGCharDetected = 0; if(lenMsg > 0 && flags & PARSE_HOSTNAME) { i = 0; - while(lenMsg > 0 && (isalnum(p2parse[i]) || p2parse[i] == '.' || p2parse[i] == '.' + while(i < lenMsg && (isalnum(p2parse[i]) || p2parse[i] == '.' || p2parse[i] == '.' || p2parse[i] == '_' || p2parse[i] == '-') && i < CONF_TAG_MAXSIZE) { bufParseHOSTNAME[i] = p2parse[i]; ++i; - --lenMsg; } if(i > 0 && p2parse[i] == ' ' && isalnum(p2parse[i-1])) { /* we got a hostname! */ p2parse += i + 1; /* "eat" it (including SP delimiter) */ + lenMsg -= i + 1; bufParseHOSTNAME[i] = '\0'; MsgSetHOSTNAME(pMsg, bufParseHOSTNAME, i); } -- cgit v1.2.3 From 5b1eb920914d97aee68b674459b68e99957c80d6 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 15 Oct 2009 13:40:10 +0200 Subject: improved imudp so that epoll can be used in more environments Fixed potential compile time problem if EPOLL_CLOEXEC is not available. --- ChangeLog | 2 ++ configure.ac | 2 +- plugins/imudp/imudp.c | 11 +++++++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index d9027d89..6e6021eb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -16,6 +16,8 @@ Version 5.3.2 [DEVEL] (rgerhards), 2009-10-?? In any case, if there is a shutdown issue, one should carefully look at this change here! - 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. - some cleanup/slight improvement: * changed imuxsock to no longer use deprecated submitAndParseMsg() IF * changed submitAndParseMsg() interface to be a wrapper around the new diff --git a/configure.ac b/configure.ac index e3f60b5c..8c782762 100644 --- a/configure.ac +++ b/configure.ac @@ -107,7 +107,7 @@ AC_TYPE_SIGNAL AC_FUNC_STAT AC_FUNC_STRERROR_R AC_FUNC_VPRINTF -AC_CHECK_FUNCS([flock basename alarm clock_gettime gethostbyname gethostname gettimeofday localtime_r memset mkdir regcomp select setid socket strcasecmp strchr strdup strerror strndup strnlen strrchr strstr strtol strtoul uname ttyname_r getline malloc_trim prctl epoll_create1 fdatasync]) +AC_CHECK_FUNCS([flock basename alarm clock_gettime gethostbyname gethostname gettimeofday localtime_r memset mkdir regcomp select setid socket strcasecmp strchr strdup strerror strndup strnlen strrchr strstr strtol strtoul uname ttyname_r getline malloc_trim prctl epoll_create epoll_create1 fdatasync]) # Check for MAXHOSTNAMELEN AC_MSG_CHECKING(for MAXHOSTNAMELEN) diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index 12946c39..3159de4f 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -293,7 +293,7 @@ finalize_it: * interface. ./configure settings control which one is used. * rgerhards, 2009-09-09 */ -#if HAVE_EPOLL_CREATE1 +#if defined(HAVE_EPOLL_CREATE1) || defined(HAVE_EPOLL_CREATE) #define NUM_EPOLL_EVENTS 10 rsRetVal rcvMainLoop() { @@ -318,7 +318,13 @@ rsRetVal rcvMainLoop() CHKmalloc(udpEPollEvt = calloc(udpLstnSocks[0], sizeof(struct epoll_event))); - efd = epoll_create1(EPOLL_CLOEXEC); +# if defined(EPOLL_CLOEXEC) && defined(HAVE_EPOLL_CREATE1) + DBGPRINTF("imudp uses epoll_create1()\n"); + efd = epoll_create1(EPOLL_CLOEXEC); +# else + DBGPRINTF("imudp uses epoll_create()\n"); + efd = epoll_create(); +# endif if(efd < 0) { DBGPRINTF("epoll_create1() could not create fd\n"); ABORT_FINALIZE(RS_RET_IO_ERROR); @@ -379,6 +385,7 @@ rsRetVal rcvMainLoop() */ bIsPermitted = 0; memset(&frominetPrev, 0, sizeof(frominetPrev)); + DBGPRINTF("imudp uses select()\n"); while(1) { /* Add the Unix Domain Sockets to the list of read -- cgit v1.2.3 From e53d91ce666453b114880e0619dd8c4d40072201 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 15 Oct 2009 18:33:33 +0200 Subject: solved a recently introduced race during input thread shutdown This was introduced when we re-enabled non-cancel thread termination a few commits ago. This code has never been released as a tarball, so that is no bugfix for a release but rather a WiP regression fix and thus does not need to be mentioned in the ChangeLog. --- ChangeLog | 10 ++----- configure.ac | 2 +- plugins/imudp/imudp.c | 8 +++--- threads.c | 80 +++++++++++++++++++++++++++++++++++++++------------ threads.h | 3 +- 5 files changed, 72 insertions(+), 31 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6e6021eb..7de6b470 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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 diff --git a/threads.c b/threads.c index a6cbc2ff..b8c19ed2 100644 --- a/threads.c +++ b/threads.c @@ -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 #include #include +#include #include #include @@ -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: */ diff --git a/threads.h b/threads.h index c37157fe..643f0107 100644 --- a/threads.h +++ b/threads.h @@ -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 */ -- cgit v1.2.3 From cd118cfcc22ea283c8d0112aeedc3f0d8b42d8a8 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 16 Oct 2009 08:41:59 +0200 Subject: bugfix: compile problem when system provided only epoll_create() I introduced that problem yesterday when I improved epoll support. --- plugins/imudp/imudp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index f8227fa7..269380cf 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -323,7 +323,7 @@ rsRetVal rcvMainLoop(thrdInfo_t *pThrd) efd = epoll_create1(EPOLL_CLOEXEC); # else DBGPRINTF("imudp uses epoll_create()\n"); - efd = epoll_create(); + efd = epoll_create(NUM_EPOLL_EVENTS); # endif if(efd < 0) { DBGPRINTF("epoll_create1() could not create fd\n"); -- cgit v1.2.3 From ba475a90cf877977be67ebb72fa844f36f338f1d Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 16 Oct 2009 09:05:21 +0200 Subject: added forgotten file --- doc/omstdout.html | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 doc/omstdout.html diff --git a/doc/omstdout.html b/doc/omstdout.html new file mode 100644 index 00000000..0bd10cfb --- /dev/null +++ b/doc/omstdout.html @@ -0,0 +1,42 @@ + + + +stdout output module (omstdout) + + +rsyslog module reference + +

stdout output module (stdout)

+

Module Name:    omstdout

+

Author: Rainer Gerhards +<rgerhards@adiscon.com>

+

Available Since: 4.1.6

+

Description:

+

This module writes any messages that are passed to it to stdout. +It was developed for the rsyslog test suite. However, there may +(limited) other uses exists. Please not that we do not put too much +effort into the quality of this module as we do not expect it to +be used in real deployments. If you do, please drop us a note so +that we can enhance its priority! +

Configuration Directives:

+
    +
  • $ActionOMStdoutArrayInterface [on|off
    +This setting instructs omstdout to use the alternate +array based method of parameter passing. If used, the values +will be output with commas between the values but no other padding bytes. +This is a test aid for the alternate calling interface. +
  • $ActionOMStdoutEnsureLFEnding [on|off
    +Makes sure that each message is written with a terminating LF. This is needed for +the automatted tests. If the message contains a trailing LF, none is added. +
+Caveats/Known Bugs: +

Currently none known. +

[rsyslog.conf overview] +[manual index] [rsyslog site]

+

This documentation is part of the +rsyslog +project.
+Copyright © 2009 by Rainer Gerhards and +Adiscon. +Released under the GNU GPL version 3 or higher.

+ -- cgit v1.2.3 From f3884d52628b4e43425adad3826c9e324cd60291 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 16 Oct 2009 09:18:51 +0200 Subject: ensure proper imudp shutdown even on a very busy system --- plugins/imudp/imudp.c | 12 +++++++----- plugins/omtesting/omtesting.c | 1 + runtime/rsyslog.h | 1 + 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index 59d23adb..5a1d9e8b 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -198,7 +198,7 @@ finalize_it: * on scheduling order. -- rgerhards, 2008-10-02 */ static inline rsRetVal -processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted, +processSocket(thrdInfo_t *pThrd, int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted, uchar *fromHost, uchar *fromHostFQDN, uchar *fromHostIP, ruleset_t *pRuleset) { DEFiRet; @@ -213,8 +213,11 @@ processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted, prop_t *propFromHostIP = NULL; char errStr[1024]; + assert(pThrd != NULL); iNbrTimeUsed = 0; while(1) { /* loop is terminated if we have a bad receive, done below in the body */ + if(pThrd->bShallStop == TRUE) + ABORT_FINALIZE(RS_RET_FORCE_TERM); socklen = sizeof(struct sockaddr_storage); lenRcvBuf = recvfrom(fd, (char*) pRcvBuf, iMaxLine, 0, (struct sockaddr *)&frominet, &socklen); if(lenRcvBuf < 0) { @@ -259,8 +262,7 @@ processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted, } } - //DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%.80s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); - DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); + DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%.80s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); if(*pbIsPermitted) { if((iTimeRequery == 0) || (iNbrTimeUsed++ % iTimeRequery) == 0) { @@ -355,7 +357,7 @@ rsRetVal rcvMainLoop(thrdInfo_t *pThrd) break; /* terminate input! */ for(i = 0 ; i < nfds ; ++i) { - processSocket(udpLstnSocks[currEvt[i].data.u64], &frominetPrev, &bIsPermitted, + processSocket(pThrd, udpLstnSocks[currEvt[i].data.u64], &frominetPrev, &bIsPermitted, fromHost, fromHostFQDN, fromHostIP, udpRulesets[currEvt[i].data.u64]); } } @@ -422,7 +424,7 @@ rsRetVal rcvMainLoop(thrdInfo_t *pThrd) for(i = 0; nfds && i < *udpLstnSocks; i++) { if(FD_ISSET(udpLstnSocks[i+1], &readfds)) { - processSocket(udpLstnSocks[i+1], &frominetPrev, &bIsPermitted, + processSocket(pThrd, udpLstnSocks[i+1], &frominetPrev, &bIsPermitted, fromHost, fromHostFQDN, fromHostIP, udpRulesets[i+1]); --nfds; /* indicate we have processed one descriptor */ } diff --git a/plugins/omtesting/omtesting.c b/plugins/omtesting/omtesting.c index 8f6cdbe5..9442f691 100644 --- a/plugins/omtesting/omtesting.c +++ b/plugins/omtesting/omtesting.c @@ -53,6 +53,7 @@ #include "dirty.h" #include "syslogd-types.h" #include "module-template.h" +#include "conf.h" #include "cfsysline.h" MODULE_TYPE_OUTPUT diff --git a/runtime/rsyslog.h b/runtime/rsyslog.h index 59e8458b..6cd4df36 100644 --- a/runtime/rsyslog.h +++ b/runtime/rsyslog.h @@ -386,6 +386,7 @@ enum rsRetVal_ /** return value. All methods return this if not specified oth RS_RET_NO_SRCNAME_TPL = -2150, /**< sourcename template was not specified where one was needed (omudpspoof spoof addr) */ RS_RET_HOST_NOT_SPECIFIED = -2151, /**< (target) host was not specified where it was needed */ RS_RET_ERR_LIBNET_INIT = -2152, /**< error initializing libnet */ + RS_RET_FORCE_TERM = -2153, /**< thread was forced to terminate be bShallShutdown, a state, not an error */ /* RainerScript error messages (range 1000.. 1999) */ RS_RET_SYSVAR_NOT_FOUND = 1001, /**< system variable could not be found (maybe misspelled) */ -- cgit v1.2.3 From 5c0920c159dc76565af344d65d0aa57d62cc389a Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 16 Oct 2009 09:24:55 +0200 Subject: cosmetic: cleanup of imfile --- plugins/imfile/imfile.c | 48 +++++++++++++----------------------------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/plugins/imfile/imfile.c b/plugins/imfile/imfile.c index 7cfde940..955d5b14 100644 --- a/plugins/imfile/imfile.c +++ b/plugins/imfile/imfile.c @@ -5,7 +5,7 @@ * * Work originally begun on 2008-02-01 by Rainer Gerhards * - * Copyright 2008 Rainer Gerhards and Adiscon GmbH. + * Copyright 2008,2009 Rainer Gerhards and Adiscon GmbH. * * This file is part of rsyslog. * @@ -213,7 +213,6 @@ static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData) } finalize_it: - /*EMPTY - just to keep the compiler happy, do NOT remove*/; /* Note: the problem above is that pthread:cleanup_pop() is a macro which * evaluates to something like "} while(0);". So the code would become * "finalize_it: }", that is a label without a statement. The C standard does @@ -243,27 +242,12 @@ finalize_it: * IMPORTANT: the calling interface of this function can NOT be modified. It actually is * called by pthreads. The provided argument is currently not being used. */ -/* ------------------------------------------------------------------------------------------ * - * DO NOT TOUCH the following code - it will soon be part of the module generation macros! */ static void inputModuleCleanup(void __attribute__((unused)) *arg) { BEGINfunc -/* END no-touch zone * - * ------------------------------------------------------------------------------------------ */ - - - - /* so far not needed */ - - - -/* ------------------------------------------------------------------------------------------ * - * DO NOT TOUCH the following code - it will soon be part of the module generation macros! */ ENDfunc } -/* END no-touch zone * - * ------------------------------------------------------------------------------------------ */ /* This function is called by the framework to gather the input. The module stays @@ -291,28 +275,22 @@ BEGINrunInput int i; int bHadFileData; /* were there at least one file with data during this run? */ CODESTARTrunInput - /* ------------------------------------------------------------------------------------------ * - * DO NOT TOUCH the following code - it will soon be part of the module generation macros! */ pthread_cleanup_push(inputModuleCleanup, NULL); - while(1) { /* endless loop - do NOT break; out of it! */ - /* END no-touch zone * - * ------------------------------------------------------------------------------------------ */ + while(1) { - do { - bHadFileData = 0; - for(i = 0 ; i < iFilPtr ; ++i) { - pollFile(&files[i], &bHadFileData); - } - } while(iFilPtr > 1 && bHadFileData == 1); /* waring: do...while()! */ + do { + bHadFileData = 0; + for(i = 0 ; i < iFilPtr ; ++i) { + pollFile(&files[i], &bHadFileData); + } + } while(iFilPtr > 1 && bHadFileData == 1); /* warning: do...while()! */ - /* Note: the additional 10ns wait is vitally important. It guards rsyslog against totally - * hogging the CPU if the users selects a polling interval of 0 seconds. It doesn't hurt any - * other valid scenario. So do not remove. -- rgerhards, 2008-02-14 - */ - srSleep(iPollInterval, 10); + /* Note: the additional 10ns wait is vitally important. It guards rsyslog against totally + * hogging the CPU if the users selects a polling interval of 0 seconds. It doesn't hurt any + * other valid scenario. So do not remove. -- rgerhards, 2008-02-14 + */ + srSleep(iPollInterval, 10); - /* ------------------------------------------------------------------------------------------ * - * DO NOT TOUCH the following code - it will soon be part of the module generation macros! */ } /*NOTREACHED*/ -- cgit v1.2.3 From d9f1a16de4723f9e096fa9abe47e8a845c815df0 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 16 Oct 2009 09:33:21 +0200 Subject: improved input thread termination for imtcp and imuxsock --- plugins/imuxsock/imuxsock.c | 3 +++ tcpsrv.c | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/plugins/imuxsock/imuxsock.c b/plugins/imuxsock/imuxsock.c index b8546ce3..a85bc4ef 100644 --- a/plugins/imuxsock/imuxsock.c +++ b/plugins/imuxsock/imuxsock.c @@ -330,6 +330,8 @@ CODESTARTrunInput break; /* terminate input! */ for (i = 0; i < nfunix && nfds > 0; i++) { + if(glbl.GetGlobalInputTermState() == 1) + ABORT_FINALIZE(RS_RET_FORCE_TERM); /* terminate input! */ if ((fd = funix[i]) != -1 && FD_ISSET(fd, &readfds)) { readSocket(fd, i); --nfds; /* indicate we have processed one */ @@ -337,6 +339,7 @@ CODESTARTrunInput } } +finalize_it: RETiRet; ENDrunInput diff --git a/tcpsrv.c b/tcpsrv.c index 49d8a099..5fe98a91 100644 --- a/tcpsrv.c +++ b/tcpsrv.c @@ -565,6 +565,8 @@ Run(tcpsrv_t *pThis) break; /* terminate input! */ for(i = 0 ; i < pThis->iLstnCurr ; ++i) { + if(glbl.GetGlobalInputTermState() == 1) + ABORT_FINALIZE(RS_RET_FORCE_TERM); CHKiRet(nssel.IsReady(pSel, pThis->ppLstn[i], NSDSEL_RD, &bIsReady, &nfds)); if(bIsReady) { DBGPRINTF("New connect on NSD %p.\n", pThis->ppLstn[i]); @@ -576,6 +578,8 @@ Run(tcpsrv_t *pThis) /* now check the sessions */ iTCPSess = TCPSessGetNxtSess(pThis, -1); while(nfds && iTCPSess != -1) { + if(glbl.GetGlobalInputTermState() == 1) + ABORT_FINALIZE(RS_RET_FORCE_TERM); CHKiRet(nssel.IsReady(pSel, pThis->pSessions[iTCPSess]->pStrm, NSDSEL_RD, &bIsReady, &nfds)); if(bIsReady) { doReceive(pThis, &pThis->pSessions[iTCPSess]); -- cgit v1.2.3 From e005c5569c3e0c7c9a098036b7ec3596c3c722b4 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 16 Oct 2009 09:42:36 +0200 Subject: some minor cleanup, consolidated some code --- plugins/immark/immark.c | 10 ++-------- threads.c | 24 ------------------------ threads.h | 1 - 3 files changed, 2 insertions(+), 33 deletions(-) diff --git a/plugins/immark/immark.c b/plugins/immark/immark.c index 19f43456..5d48369e 100644 --- a/plugins/immark/immark.c +++ b/plugins/immark/immark.c @@ -42,6 +42,7 @@ #include "module-template.h" #include "errmsg.h" #include "msg.h" +#include "srUtils.h" #include "glbl.h" MODULE_TYPE_INPUT @@ -80,20 +81,13 @@ CODESTARTrunInput * right into the sleep below. */ while(1) { - /* we do not need to handle the RS_RET_TERMINATE_NOW case any - * special because we just need to terminate. This may be different - * if a cleanup is needed. But for now, we can just use CHKiRet(). - * rgerhards, 2007-12-17 - */ - CHKiRet(thrdSleep(pThrd, iMarkMessagePeriod, 0)); /* seconds, micro seconds */ + srSleep(iMarkMessagePeriod, 0); /* seconds, micro seconds */ if(glbl.GetGlobalInputTermState() == 1) break; /* terminate input! */ logmsgInternal(NO_ERRCODE, LOG_INFO, (uchar*)"-- MARK --", MARK); } -finalize_it: - return iRet; ENDrunInput diff --git a/threads.c b/threads.c index b8c19ed2..ccc80816 100644 --- a/threads.c +++ b/threads.c @@ -250,29 +250,5 @@ rsRetVal thrdExit(void) } -/* thrdSleep() - a fairly portable way to put a thread to sleep. It - * will wake up when - * a) the wake-time is over - * b) the thread shall be terminated - * Returns RS_RET_OK if all went well, RS_RET_TERMINATE_NOW if the calling - * thread shall be terminated and any other state if an error happened. - * rgerhards, 2007-12-17 - */ -rsRetVal -thrdSleep(thrdInfo_t *pThis, int iSeconds, int iuSeconds) -{ - DEFiRet; - struct timeval tvSelectTimeout; - - assert(pThis != NULL); - tvSelectTimeout.tv_sec = iSeconds; - tvSelectTimeout.tv_usec = iuSeconds; /* micro seconds */ - select(1, NULL, NULL, NULL, &tvSelectTimeout); - if(pThis->bShallStop) - iRet = RS_RET_TERMINATE_NOW; - RETiRet; -} - - /* vi:set ai: */ diff --git a/threads.h b/threads.h index 643f0107..1cac02b5 100644 --- a/threads.h +++ b/threads.h @@ -41,7 +41,6 @@ rsRetVal thrdInit(void); rsRetVal thrdTerminate(thrdInfo_t *pThis); rsRetVal thrdTerminateAll(void); rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), rsRetVal(*afterRun)(thrdInfo_t *), bool); -rsRetVal thrdSleep(thrdInfo_t *pThis, int iSeconds, int iuSeconds); /* macros (replace inline functions) */ -- cgit v1.2.3