From 40814bb0307f9d8536b3dc4a28b94f80a361b41d Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 27 Oct 2010 10:10:47 +0200 Subject: imfile improvements - added the $InputFilePersistStateInterval config directive to imfile - changed imfile so that the state file is never deleted (makes imfile more robust in regard to fatal failures) --- ChangeLog | 6 ++++++ configure.ac | 2 +- doc/imfile.html | 10 ++++++++++ plugins/imfile/imfile.c | 20 +++++++++++++++++--- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index cf752df5..0dea5306 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ --------------------------------------------------------------------------- Version 5.6.0 [V5-STABLE] (rgerhards), 2010-10-19 +- private build +- added the $InputFilePersistStateInterval config directive to imfile +- changed imfile so that the state file is never deleted (makes imfile + more robust in regard to fatal failures) +--------------------------------------------------------------------------- +Version 5.6.0 [V5-STABLE] (rgerhards), 2010-10-19 This release brings all changes and enhancements of the 5.5.x series to the v5-stable branch. diff --git a/configure.ac b/configure.ac index 7b9a1949..410046f5 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.6.0],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[5.6.0a],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) diff --git a/doc/imfile.html b/doc/imfile.html index af0413dd..a7ac5a49 100644 --- a/doc/imfile.html +++ b/doc/imfile.html @@ -86,6 +86,16 @@ level may be needed. Even if you need quick response, 1 seconds should be well enough. Please note that imfile keeps reading files as long as there is any data in them. So a "polling sleep" will only happen when nothing is left to be processed. +
  • $InputFilePersistStateInterval [lines]
    +Available in 4.7.3+
    +Specifies how often the state file shall be written when processing the input +file. The default value is 0, which means a new state file is only written when +the monitored files is being closed (end of rsyslogd execution). Any other +value n means that the state file is written every time n file lines have +been processed. This setting can be used to guard against message duplication due +to fatal errors (like power fail). Note that this setting affects imfile +performance, especially when set to a low value. Frequently writing the state +file is very time consuming. Caveats/Known Bugs:

    So far, only 100 files can be monitored. If more are needed, diff --git a/plugins/imfile/imfile.c b/plugins/imfile/imfile.c index 6c2a64de..686fa048 100644 --- a/plugins/imfile/imfile.c +++ b/plugins/imfile/imfile.c @@ -47,6 +47,7 @@ #include "datetime.h" #include "unicode-helper.h" #include "prop.h" +#include "stringbuf.h" MODULE_TYPE_INPUT /* must be present for input modules, do not remove */ @@ -67,15 +68,21 @@ typedef struct fileInfo_s { uchar *pszStateFile; /* file in which state between runs is to be stored */ int iFacility; int iSeverity; + int nRecords; /**< How many records did we process before persisting the stream? */ + int iPersistStateInterval; /**< how often should state be persisted? (0=on close only) */ strm_t *pStrm; /* its stream (NULL if not assigned) */ } fileInfo_t; +/* forward definitions */ +static rsRetVal persistStrmState(fileInfo_t *pInfo); + /* config variables */ static uchar *pszFileName = NULL; static uchar *pszFileTag = NULL; static uchar *pszStateFile = NULL; static int iPollInterval = 10; /* number of seconds to sleep when there was no file activity */ +static int iPersistStateInterval = 0; /* how often if state file to be persisted? (default 0->never) */ static int iFacility = 128; /* local0 */ static int iSeverity = 5; /* notice, as of rfc 3164 */ @@ -153,10 +160,9 @@ openFile(fileInfo_t *pThis) CHKiRet(strm.SeekCurrOffs(pThis->pStrm)); - /* OK, we could successfully read the file, so we now can request that it be deleted. - * If we need it again, it will be written on the next shutdown. + /* note: we do not delete the state file, so that the last position remains + * known even in the case that rsyslogd aborts for some reason (like powerfail) */ - psSF->bDeleteOnClose = 1; finalize_it: if(psSF != NULL) @@ -210,6 +216,10 @@ static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData) *pbHadFileData = 1; /* this is just a flag, so set it and forget it */ CHKiRet(enqLine(pThis, pCStr)); /* process line */ rsCStrDestruct(&pCStr); /* discard string (must be done by us!) */ + if(pThis->iPersistStateInterval > 0 && pThis->nRecords++ >= pThis->iPersistStateInterval) { + persistStrmState(pThis); + pThis->nRecords = 0; + } } finalize_it: @@ -477,6 +487,7 @@ static rsRetVal addMonitor(void __attribute__((unused)) *pVal, uchar *pNewVal) pThis->iSeverity = iSeverity; pThis->iFacility = iFacility; + pThis->iPersistStateInterval = iPersistStateInterval; } else { errmsg.LogError(0, RS_RET_OUT_OF_DESRIPTORS, "Too many file monitors configured - ignoring this one"); ABORT_FINALIZE(RS_RET_OUT_OF_DESRIPTORS); @@ -522,6 +533,9 @@ CODEmodInit_QueryRegCFSLineHdlr NULL, &iFacility, STD_LOADABLE_MODULE_ID)); CHKiRet(omsdRegCFSLineHdlr((uchar *)"inputfilepollinterval", 0, eCmdHdlrInt, NULL, &iPollInterval, STD_LOADABLE_MODULE_ID)); + CHKiRet(omsdRegCFSLineHdlr((uchar *)"inputfilepersiststateinterval", 0, eCmdHdlrInt, + + NULL, &iPersistStateInterval, STD_LOADABLE_MODULE_ID)); /* that command ads a new file! */ CHKiRet(omsdRegCFSLineHdlr((uchar *)"inputrunfilemonitor", 0, eCmdHdlrGetWord, addMonitor, NULL, STD_LOADABLE_MODULE_ID)); -- cgit v1.2.3 From 7904ef4e377dda677a9e665e20401d8753535fca Mon Sep 17 00:00:00 2001 From: varmojfekoj Date: Fri, 5 Nov 2010 10:04:33 +0100 Subject: bugfix: compile failed with --enable-unlimited-select Signed-off-by: Rainer Gerhards --- ChangeLog | 4 ++++ runtime/nsdpoll_ptcp.c | 1 - runtime/unlimited_select.h | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index cf752df5..8b735a94 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,8 @@ --------------------------------------------------------------------------- +Version 5.6.1 [V5-BETA] (rgerhards), 2010-??-?? +- bugfix: compile failed with --enable-unlimited-select + thanks varmojfekoj for the patch +--------------------------------------------------------------------------- Version 5.6.0 [V5-STABLE] (rgerhards), 2010-10-19 This release brings all changes and enhancements of the 5.5.x series diff --git a/runtime/nsdpoll_ptcp.c b/runtime/nsdpoll_ptcp.c index 51006707..bc374c60 100644 --- a/runtime/nsdpoll_ptcp.c +++ b/runtime/nsdpoll_ptcp.c @@ -42,7 +42,6 @@ #include "nspoll.h" #include "nsd_ptcp.h" #include "nsdpoll_ptcp.h" -#include "unlimited_select.h" /* static data */ DEFobjStaticHelpers diff --git a/runtime/unlimited_select.h b/runtime/unlimited_select.h index 32dadc03..3fa7eb06 100644 --- a/runtime/unlimited_select.h +++ b/runtime/unlimited_select.h @@ -23,6 +23,7 @@ */ #ifndef UNLIMITED_SELECT_H_INCLUDED +#define UNLIMITED_SELECT_H_INCLUDED #include #include -- cgit v1.2.3 From 71b8b60b220945aa0c2b541bf144182e2bb6e032 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 5 Nov 2010 10:41:44 +0100 Subject: bugfix: segfault when an *empty* template was used Bug: http://bugzilla.adiscon.com/show_bug.cgi?id=206 Thanks to David Hill for alerting us. --- ChangeLog | 3 +++ template.c | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8b735a94..e78dcd48 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ --------------------------------------------------------------------------- Version 5.6.1 [V5-BETA] (rgerhards), 2010-??-?? +- bugfix: segfault when an *empty* template was used + Bug: http://bugzilla.adiscon.com/show_bug.cgi?id=206 + Thanks to David Hill for alerting us. - bugfix: compile failed with --enable-unlimited-select thanks varmojfekoj for the patch --------------------------------------------------------------------------- diff --git a/template.c b/template.c index c46d144e..06949e45 100644 --- a/template.c +++ b/template.c @@ -85,7 +85,7 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * { DEFiRet; struct templateEntry *pTpe; - int iBuf; + size_t iBuf; unsigned short bMustBeFreed; uchar *pVal; size_t iLenVal; @@ -141,7 +141,15 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * pTpe = pTpe->pNext; } - (*ppBuf)[iBuf] = '\0'; /* space was reserved above (see copy) */ + if(iBuf == *pLenBuf) { + /* in the weired case of an *empty* template, this can happen. + * it is debatable if we should really fix it here or simply + * forbid that case. However, performance toll is minimal, so + * I tend to permit it. -- 201011-05 rgerhards + */ + CHKiRet(ExtendBuf(ppBuf, pLenBuf, iBuf + 1)); + } + (*ppBuf)[iBuf] = '\0'; finalize_it: RETiRet; -- cgit v1.2.3 From cb04fc3972ef983b9dd6a550e275639e73502527 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 5 Nov 2010 11:02:14 +0100 Subject: permitted imptcp to work on systems which support epoll(), but not epoll_create(). Bug: http://bugzilla.adiscon.com/show_bug.cgi?id=204 Thanks to Nicholas Brink for reporting this problem. --- ChangeLog | 4 ++++ plugins/imptcp/imptcp.c | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index e78dcd48..469ee115 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ --------------------------------------------------------------------------- Version 5.6.1 [V5-BETA] (rgerhards), 2010-??-?? +- permitted imptcp to work on systems which support epoll(), but not + epoll_create(). + Bug: http://bugzilla.adiscon.com/show_bug.cgi?id=204 + Thanks to Nicholas Brink for reporting this problem. - bugfix: segfault when an *empty* template was used Bug: http://bugzilla.adiscon.com/show_bug.cgi?id=206 Thanks to David Hill for alerting us. diff --git a/plugins/imptcp/imptcp.c b/plugins/imptcp/imptcp.c index 9b24dbc2..6b14a80e 100644 --- a/plugins/imptcp/imptcp.c +++ b/plugins/imptcp/imptcp.c @@ -1039,7 +1039,14 @@ CODESTARTwillRun ABORT_FINALIZE(RS_RET_NO_RUN); } - if((epollfd = epoll_create1(EPOLL_CLOEXEC)) < 0) { +# if defined(EPOLL_CLOEXEC) && defined(HAVE_EPOLL_CREATE1) + DBGPRINTF("imptcp uses epoll_create1()\n"); + epollfd = epoll_create1(EPOLL_CLOEXEC); +# else + DBGPRINTF("imptcp uses epoll_create()\n"); + epollfd = epoll_create(NUM_EPOLL_EVENTS); +# endif + if(epollfd < 0) { errmsg.LogError(0, RS_RET_EPOLL_CR_FAILED, "error: epoll_create() failed"); ABORT_FINALIZE(RS_RET_NO_RUN); } -- cgit v1.2.3 From 7f6b471bf56d1ced716a21bc3a7444b2fab985ea Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 5 Nov 2010 11:44:41 +0100 Subject: provide clear error message if platform does not support imptcp --- plugins/imptcp/imptcp.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugins/imptcp/imptcp.c b/plugins/imptcp/imptcp.c index 6b14a80e..51f33bab 100644 --- a/plugins/imptcp/imptcp.c +++ b/plugins/imptcp/imptcp.c @@ -30,6 +30,13 @@ * A copy of the GPL can be found in the file "COPYING" in this distribution. */ #include "config.h" +#if !defined(HAVE_EPOLL_CREATE) +# error imptcp requires OS support for epoll - can not build + /* imptcp gains speed by using modern Linux capabilities. As such, + * it can only be build on platforms supporting the epoll API. + */ +#endif + #include #include #include -- cgit v1.2.3 From c470879eff9256612cfcff4938ed81425b3cc3c6 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 5 Nov 2010 12:56:07 +0100 Subject: bugfix: testbench failed if imptcp was not enabled --- ChangeLog | 1 + tests/Makefile.am | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 469ee115..fb1628e8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ Version 5.6.1 [V5-BETA] (rgerhards), 2010-??-?? epoll_create(). Bug: http://bugzilla.adiscon.com/show_bug.cgi?id=204 Thanks to Nicholas Brink for reporting this problem. +- bugfix: testbench failed if imptcp was not enabled - bugfix: segfault when an *empty* template was used Bug: http://bugzilla.adiscon.com/show_bug.cgi?id=206 Thanks to David Hill for alerting us. diff --git a/tests/Makefile.am b/tests/Makefile.am index a0231503..3c00c24a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -14,9 +14,6 @@ TESTS = $(TESTRUNS) cfg.sh \ manytcp.sh \ rsf_getenv.sh \ manyptcp.sh \ - imptcp_large.sh \ - imptcp_addtlframedelim.sh \ - imptcp_conndrop.sh \ imtcp_conndrop.sh \ imtcp_addtlframedelim.sh \ sndrcv.sh \ @@ -51,6 +48,13 @@ TESTS = $(TESTRUNS) cfg.sh \ dircreate_off.sh \ queue-persist.sh +if ENABLE_IMPTCP +TESTS += \ + imptcp_large.sh \ + imptcp_addtlframedelim.sh \ + imptcp_conndrop.sh +endif + if ENABLE_OMUDPSPOOF TESTS += sndrcv_omudpspoof.sh \ sndrcv_omudpspoof_nonstdpt.sh -- cgit v1.2.3 From 925504d565c6cf4a712dd8c8217891662aaf639e Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 24 Nov 2010 11:14:21 +0100 Subject: bugfix(important): problem in TLS handling could cause rsyslog to loop ... in a tight loop, effectively disabling functionality and bearing the risk of unresponsiveness of the whole system. Bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=194 --- ChangeLog | 6 ++++++ configure.ac | 2 +- runtime/nsdsel_gtls.c | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 6d67a35e..e17ef35d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,10 @@ --------------------------------------------------------------------------- +Version 3.22.3 [v3-stable] (rgerhards), 2010-11-24 +- bugfix(important): problem in TLS handling could cause rsyslog to loop + in a tight loop, effectively disabling functionality and bearing the + risk of unresponsiveness of the whole system. + Bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=194 +--------------------------------------------------------------------------- Version 3.22.2 [v3-stable] (rgerhards), 2010-08-05 - bugfix: comment char ('#') in literal terminated script parsing and thus could not be used. diff --git a/configure.ac b/configure.ac index ce6d6165..91c3cbfa 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],[3.22.2],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[3.22.3],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([ChangeLog]) AC_CONFIG_MACRO_DIR([m4]) diff --git a/runtime/nsdsel_gtls.c b/runtime/nsdsel_gtls.c index c3a93bee..1a389a00 100644 --- a/runtime/nsdsel_gtls.c +++ b/runtime/nsdsel_gtls.c @@ -76,6 +76,9 @@ Add(nsdsel_t *pNsdsel, nsd_t *pNsd, nsdsel_waitOp_t waitOp) if(pNsdGTLS->iMode == 1) { if(waitOp == NSDSEL_RD && gtlsHasRcvInBuffer(pNsdGTLS)) { ++pThis->iBufferRcvReady; + dbgprintf("nsdsel_gtls: data already present in buffer, initiating " + "dummy select %p->iBufferRcvReady=%d\n", + pThis, pThis->iBufferRcvReady); FINALIZE; } if(pNsdGTLS->rtryCall != gtlsRtry_None) { @@ -109,6 +112,7 @@ Select(nsdsel_t *pNsdsel, int *piNumReady) if(pThis->iBufferRcvReady > 0) { /* we still have data ready! */ *piNumReady = pThis->iBufferRcvReady; + dbgprintf("nsdsel_gtls: doing dummy select, data present\n"); } else { iRet = nsdsel_ptcp.Select(pThis->pTcp, piNumReady); } @@ -190,6 +194,9 @@ IsReady(nsdsel_t *pNsdsel, nsd_t *pNsd, nsdsel_waitOp_t waitOp, int *pbIsReady) if(pNsdGTLS->iMode == 1) { if(waitOp == NSDSEL_RD && gtlsHasRcvInBuffer(pNsdGTLS)) { *pbIsReady = 1; + --pThis->iBufferRcvReady; /* one "pseudo-read" less */ + dbgprintf("nsdl_gtls: dummy read, decermenting %p->iBufRcvReady, now %d\n", + pThis, pThis->iBufferRcvReady); FINALIZE; } if(pNsdGTLS->rtryCall != gtlsRtry_None) { @@ -200,6 +207,16 @@ IsReady(nsdsel_t *pNsdsel, nsd_t *pNsd, nsdsel_waitOp_t waitOp, int *pbIsReady) *pbIsReady = 0; FINALIZE; } + /* now we must ensure that we do not fall back to PTCP if we have + * done a "dummy" select. In that case, we know when the predicate + * is not matched here, we do not have data available for this + * socket. -- rgerhards, 2010-11-20 + */ + if(pThis->iBufferRcvReady) { + dbgprintf("nsd_gtls: dummy read, buffer not available for this FD\n"); + *pbIsReady = 0; + FINALIZE; + } } CHKiRet(nsdsel_ptcp.IsReady(pThis->pTcp, pNsdGTLS->pTcp, waitOp, pbIsReady)); -- cgit v1.2.3 From e4ac19d8ff2a7ef7698048e7c732f4e404f007b8 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 24 Nov 2010 16:27:14 +0100 Subject: final cleanup for 5.6.1 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index f80d77e1..506650d6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,5 @@ --------------------------------------------------------------------------- -Version 5.6.1 [V5-BETA] (rgerhards), 2010-??-?? +Version 5.6.1 [V5-STABLE] (rgerhards), 2010-11-24 - bugfix(important): problem in TLS handling could cause rsyslog to loop in a tight loop, effectively disabling functionality and bearing the risk of unresponsiveness of the whole system. -- cgit v1.2.3 From e4e263243ae1004380e38e638a6d3d3759c5a90c Mon Sep 17 00:00:00 2001 From: David Hill Date: Thu, 25 Nov 2010 14:51:02 +0100 Subject: fixed compile problem in imptcp if no epoll_create1() is present Signed-off-by: Rainer Gerhards --- plugins/imptcp/imptcp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/imptcp/imptcp.c b/plugins/imptcp/imptcp.c index 51f33bab..a9b0a1d5 100644 --- a/plugins/imptcp/imptcp.c +++ b/plugins/imptcp/imptcp.c @@ -1035,6 +1035,7 @@ ENDrunInput /* initialize and return if will run or not */ +#define NUM_EPOLL_EVENTS 10 BEGINwillRun CODESTARTwillRun /* first apply some config settings */ -- cgit v1.2.3 From 775464beaa6610a28f574e40eb94393959cdab2b Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 25 Nov 2010 14:52:38 +0100 Subject: streamlined epoll_create() code a little bit --- plugins/imptcp/imptcp.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/imptcp/imptcp.c b/plugins/imptcp/imptcp.c index a9b0a1d5..6449ad62 100644 --- a/plugins/imptcp/imptcp.c +++ b/plugins/imptcp/imptcp.c @@ -1035,7 +1035,6 @@ ENDrunInput /* initialize and return if will run or not */ -#define NUM_EPOLL_EVENTS 10 BEGINwillRun CODESTARTwillRun /* first apply some config settings */ @@ -1052,7 +1051,11 @@ CODESTARTwillRun epollfd = epoll_create1(EPOLL_CLOEXEC); # else DBGPRINTF("imptcp uses epoll_create()\n"); - epollfd = epoll_create(NUM_EPOLL_EVENTS); + /* reading the docs, the number of epoll events passed to + * epoll_create() seems not to be used at all in kernels. So + * we just provide "a" number, happens to be 10. + */ + epollfd = epoll_create(10); # endif if(epollfd < 0) { errmsg.LogError(0, RS_RET_EPOLL_CR_FAILED, "error: epoll_create() failed"); -- cgit v1.2.3 From 64ecd12021dad06df365f94f7fddd44ebfbd0eb2 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 25 Nov 2010 15:06:49 +0100 Subject: forgot to add to ChangeLog --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index b605aaaa..602204d9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ --------------------------------------------------------------------------- Version 5.6.1 [V5-STABLE] (rgerhards), 2010-11-24 +- bugfix: compile failed on systems without epoll_create1() + Thanks to David Hill for providing a fix. +--------------------------------------------------------------------------- +Version 5.6.1 [V5-STABLE] (rgerhards), 2010-11-24 - bugfix(important): problem in TLS handling could cause rsyslog to loop in a tight loop, effectively disabling functionality and bearing the risk of unresponsiveness of the whole system. -- cgit v1.2.3 From afafd9e0d7b333c54613670f4b9dbe3ae90ec51d Mon Sep 17 00:00:00 2001 From: Chris Metcalf Date: Thu, 25 Nov 2010 15:51:49 +0100 Subject: bugfix: atomic increment for msg object may not work correct on all platforms. Signed-off-by: Rainer Gerhards --- ChangeLog | 4 +++- runtime/msg.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 602204d9..a86a5d8e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,9 @@ --------------------------------------------------------------------------- -Version 5.6.1 [V5-STABLE] (rgerhards), 2010-11-24 +Version 5.6.2 [V5-STABLE] (rgerhards), 2010-11-?? - bugfix: compile failed on systems without epoll_create1() Thanks to David Hill for providing a fix. +- bugfix: atomic increment for msg object may not work correct on all + platforms. Thanks to Chris Metcalf for the patch --------------------------------------------------------------------------- Version 5.6.1 [V5-STABLE] (rgerhards), 2010-11-24 - bugfix(important): problem in TLS handling could cause rsyslog to loop diff --git a/runtime/msg.h b/runtime/msg.h index d42f1de2..4897959c 100644 --- a/runtime/msg.h +++ b/runtime/msg.h @@ -60,8 +60,8 @@ struct msg { flowControl_t flowCtlType; /**< type of flow control we can apply, for enqueueing, needs not to be persisted because once data has entered the queue, this property is no longer needed. */ pthread_mutex_t mut; + int iRefCount; /* reference counter (0 = unused) */ sbool bDoLock; /* use the mutex? */ - short iRefCount; /* reference counter (0 = unused) */ short iSeverity; /* the severity 0..7 */ short iFacility; /* Facility code 0 .. 23*/ short offAfterPRI; /* offset, at which raw message WITHOUT PRI part starts in pszRawMsg */ -- cgit v1.2.3 From a3c81f500a4d952ce93162a730cadee8fbc8116b Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 25 Nov 2010 17:20:55 +0100 Subject: bugfix: replacements for atomic operations for non-int sized types had problems. At least one instance of that problem could potentially lead to abort (inside omfile). --- ChangeLog | 3 ++ action.c | 2 +- runtime/Makefile.am | 1 + runtime/atomic.h | 45 ++++++++++++++-- runtime/msg.c | 2 +- runtime/rsyslog.h | 121 +----------------------------------------- runtime/typedefs.h | 147 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/omfile.c | 6 ++- 8 files changed, 201 insertions(+), 126 deletions(-) create mode 100644 runtime/typedefs.h diff --git a/ChangeLog b/ChangeLog index a86a5d8e..2ed424c9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,9 @@ Version 5.6.2 [V5-STABLE] (rgerhards), 2010-11-?? Thanks to David Hill for providing a fix. - bugfix: atomic increment for msg object may not work correct on all platforms. Thanks to Chris Metcalf for the patch +- bugfix: replacements for atomic operations for non-int sized types had + problems. At least one instance of that problem could potentially lead + to abort (inside omfile). --------------------------------------------------------------------------- Version 5.6.1 [V5-STABLE] (rgerhards), 2010-11-24 - bugfix(important): problem in TLS handling could cause rsyslog to loop diff --git a/action.c b/action.c index f3e26b13..0d298673 100644 --- a/action.c +++ b/action.c @@ -1388,7 +1388,7 @@ doSubmitToActionQNotAllMarkBatch(action_t *pAction, batch_t *pBatch) } else { bProcessMarkMsgs = 1; } - } while(ATOMIC_CAS(&pAction->f_time, lastAct, + } while(ATOMIC_CAS_time_t(&pAction->f_time, lastAct, ((msg_t*)(pBatch->pElem[i].pUsrp))->ttGenTime, &pAction->mutCAS) == 0); } if(bProcessMarkMsgs) { diff --git a/runtime/Makefile.am b/runtime/Makefile.am index f7db3e35..b700eae8 100644 --- a/runtime/Makefile.am +++ b/runtime/Makefile.am @@ -7,6 +7,7 @@ pkglib_LTLIBRARIES = librsyslog_la_SOURCES = \ rsyslog.c \ rsyslog.h \ + typedefs.h \ unicode-helper.h \ atomic.h \ batch.h \ diff --git a/runtime/atomic.h b/runtime/atomic.h index da544c4b..790cb1c6 100644 --- a/runtime/atomic.h +++ b/runtime/atomic.h @@ -33,6 +33,8 @@ */ #ifndef INCLUDED_ATOMIC_H #define INCLUDED_ATOMIC_H +#include +#include "typedefs.h" /* for this release, we disable atomic calls because there seem to be some * portability problems and we can not fix that without destabilizing the build. @@ -42,7 +44,9 @@ # define ATOMIC_SUB(data, val, phlpmut) __sync_fetch_and_sub(data, val) # define ATOMIC_ADD(data, val) __sync_fetch_and_add(&(data), val) # define ATOMIC_INC(data, phlpmut) ((void) __sync_fetch_and_add(data, 1)) -# define ATOMIC_INC_AND_FETCH(data, phlpmut) __sync_fetch_and_add(data, 1) +# define ATOMIC_INC_AND_FETCH_int(data, phlpmut) __sync_fetch_and_add(data, 1) +# define ATOMIC_INC_AND_FETCH_unsigned(data, phlpmut) __sync_fetch_and_add(data, 1) +# define ATOMIC_INC_AND_FETCH_uint64(data, phlpmut) __sync_fetch_and_add(data, 1) # define ATOMIC_DEC(data, phlpmut) ((void) __sync_sub_and_fetch(data, 1)) # define ATOMIC_DEC_AND_FETCH(data, phlpmut) __sync_sub_and_fetch(data, 1) # define ATOMIC_FETCH_32BIT(data, phlpmut) ((unsigned) __sync_fetch_and_and(data, 0xffffffff)) @@ -51,6 +55,7 @@ # define ATOMIC_STORE_1_TO_INT(data, phlpmut) __sync_fetch_and_or(data, 1) # define ATOMIC_STORE_INT_TO_INT(data, val) __sync_fetch_and_or(&(data), (val)) # define ATOMIC_CAS(data, oldVal, newVal, phlpmut) __sync_bool_compare_and_swap(data, (oldVal), (newVal)) +# define ATOMIC_CAS_time_t(data, oldVal, newVal, phlpmut) __sync_bool_compare_and_swap(data, (oldVal), (newVal)) # define ATOMIC_CAS_VAL(data, oldVal, newVal, phlpmut) __sync_val_compare_and_swap(data, (oldVal), (newVal)); /* functions below are not needed if we have atomics */ @@ -104,6 +109,20 @@ return(bSuccess); } + static inline int + ATOMIC_CAS_time_t(time_t *data, time_t oldVal, time_t newVal, pthread_mutex_t *phlpmut) { + int bSuccess; + pthread_mutex_lock(phlpmut); + if(*data == oldVal) { + *data = newVal; + bSuccess = 1; + } else { + bSuccess = 0; + } + pthread_mutex_unlock(phlpmut); + return(bSuccess); + } + static inline int ATOMIC_CAS_VAL(int *data, int oldVal, int newVal, pthread_mutex_t *phlpmut) { @@ -124,7 +143,7 @@ } static inline int - ATOMIC_INC_AND_FETCH(int *data, pthread_mutex_t *phlpmut) { + ATOMIC_INC_AND_FETCH_int(int *data, pthread_mutex_t *phlpmut) { int val; pthread_mutex_lock(phlpmut); val = ++(*data); @@ -132,6 +151,24 @@ return(val); } + static inline unsigned + ATOMIC_INC_AND_FETCH_unsigned(unsigned *data, pthread_mutex_t *phlpmut) { + unsigned val; + pthread_mutex_lock(phlpmut); + val = ++(*data); + pthread_mutex_unlock(phlpmut); + return(val); + } + + static inline unsigned + ATOMIC_INC_AND_FETCH_uint64(uint64 *data, pthread_mutex_t *phlpmut) { + uint64 val; + pthread_mutex_lock(phlpmut); + val = ++(*data); + pthread_mutex_unlock(phlpmut); + return(val); + } + static inline int ATOMIC_DEC_AND_FETCH(int *data, pthread_mutex_t *phlpmut) { int val; @@ -158,7 +195,9 @@ } #if 0 # warning "atomic builtins not available, using nul operations - rsyslogd will probably be racy!" -# define ATOMIC_INC_AND_FETCH(data) (++(data)) +# define ATOMIC_INC_AND_FETCH_int(data) (++(data)) +# define ATOMIC_INC_AND_FETCH_unsigned(data) (++(data)) +# define ATOMIC_INC_AND_FETCH_uint64(data) (++(data)) # define ATOMIC_STORE_1_TO_32BIT(data) (data) = 1 // TODO: del #endif # define DEF_ATOMIC_HELPER_MUT(x) pthread_mutex_t x diff --git a/runtime/msg.c b/runtime/msg.c index f575b86b..82565f18 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -851,7 +851,7 @@ CODESTARTobjDestruct(msg) * that we trim too often when the counter wraps. */ static unsigned iTrimCtr = 1; - currCnt = ATOMIC_INC_AND_FETCH(&iTrimCtr, &mutTrimCtr); + currCnt = ATOMIC_INC_AND_FETCH_unsigned(&iTrimCtr, &mutTrimCtr); if(currCnt % 100000 == 0) { malloc_trim(128*1024); } diff --git a/runtime/rsyslog.h b/runtime/rsyslog.h index 34eaedca..3e6ac4af 100644 --- a/runtime/rsyslog.h +++ b/runtime/rsyslog.h @@ -25,6 +25,7 @@ */ #ifndef INCLUDED_RSYSLOG_H #define INCLUDED_RSYSLOG_H +#include "typedefs.h" /* ############################################################# * * # Some constant values # * @@ -94,126 +95,6 @@ #define CORE_FEATURE_BATCHING 1 /*#define CORE_FEATURE_whatever 2 ... and so on ... */ -/* under Solaris (actually only SPARC), we need to redefine some types - * to be void, so that we get void* pointers. Otherwise, we will see - * alignment errors. - */ -/* some universal fixed size integer defines ... */ -typedef long long int64; -typedef long long unsigned uint64; -typedef int64 number_t; /* type to use for numbers - TODO: maybe an autoconf option? */ -typedef char intTiny; /* 0..127! */ -typedef unsigned char uintTiny; /* 0..255! */ - -/* define some base data types */ - -typedef unsigned char uchar;/* get rid of the unhandy "unsigned char" */ -typedef struct aUsrp_s aUsrp_t; -typedef struct thrdInfo thrdInfo_t; -typedef struct obj_s obj_t; -typedef struct ruleset_s ruleset_t; -typedef struct rule_s rule_t; -//typedef struct filed selector_t;/* TODO: this so far resides in syslogd.c, think about modularization */ -typedef struct NetAddr netAddr_t; -typedef struct netstrms_s netstrms_t; -typedef struct netstrm_s netstrm_t; -typedef struct nssel_s nssel_t; -typedef struct nspoll_s nspoll_t; -typedef enum nsdsel_waitOp_e nsdsel_waitOp_t; -typedef struct nsd_ptcp_s nsd_ptcp_t; -typedef struct nsd_gtls_s nsd_gtls_t; -typedef struct nsd_gsspi_s nsd_gsspi_t; -typedef struct nsd_nss_s nsd_nss_t; -typedef struct nsdsel_ptcp_s nsdsel_ptcp_t; -typedef struct nsdsel_gtls_s nsdsel_gtls_t; -typedef struct nsdpoll_ptcp_s nsdpoll_ptcp_t; -typedef struct wti_s wti_t; -typedef struct msg msg_t; -typedef struct queue_s qqueue_t; -typedef struct prop_s prop_t; -typedef struct interface_s interface_t; -typedef struct objInfo_s objInfo_t; -typedef enum rsRetVal_ rsRetVal; /**< friendly type for global return value */ -typedef rsRetVal (*errLogFunc_t)(uchar*); /* this is a trick to store a function ptr to a function returning a function ptr... */ -typedef struct permittedPeers_s permittedPeers_t; /* this should go away in the long term -- rgerhards, 2008-05-19 */ -typedef struct permittedPeerWildcard_s permittedPeerWildcard_t; /* this should go away in the long term -- rgerhards, 2008-05-19 */ -typedef struct tcpsrv_s tcpsrv_t; -typedef struct tcps_sess_s tcps_sess_t; -typedef struct strmsrv_s strmsrv_t; -typedef struct strms_sess_s strms_sess_t; -typedef struct vmstk_s vmstk_t; -typedef struct batch_obj_s batch_obj_t; -typedef struct batch_s batch_t; -typedef struct wtp_s wtp_t; -typedef struct modInfo_s modInfo_t; -typedef struct parser_s parser_t; -typedef struct parserList_s parserList_t; -typedef struct strgen_s strgen_t; -typedef struct strgenList_s strgenList_t; -typedef rsRetVal (*prsf_t)(struct vmstk_s*, int); /* pointer to a RainerScript function */ -typedef uint64 qDeqID; /* queue Dequeue order ID. 32 bits is considered dangerously few */ - -typedef struct tcpLstnPortList_s tcpLstnPortList_t; // TODO: rename? -typedef struct strmLstnPortList_s strmLstnPortList_t; // TODO: rename? - -/* under Solaris (actually only SPARC), we need to redefine some types - * to be void, so that we get void* pointers. Otherwise, we will see - * alignment errors. - */ -#ifdef OS_SOLARIS - typedef void * obj_t_ptr; - typedef void nsd_t; - typedef void nsdsel_t; - typedef void nsdpoll_t; -#else - typedef obj_t *obj_t_ptr; - typedef obj_t nsd_t; - typedef obj_t nsdsel_t; - typedef obj_t nsdpoll_t; -#endif - - -#ifdef __hpux -typedef unsigned int u_int32_t; /* TODO: is this correct? */ -typedef int socklen_t; -#endif - -typedef struct epoll_event epoll_event_t; - -typedef char sbool; /* (small bool) I intentionally use char, to keep it slim so that many fit into the CPU cache! */ - -/* settings for flow control - * TODO: is there a better place for them? -- rgerhards, 2008-03-14 - */ -typedef enum { - eFLOWCTL_NO_DELAY = 0, /**< UDP and other non-delayable sources */ - eFLOWCTL_LIGHT_DELAY = 1, /**< some light delay possible, but no extended period of time */ - eFLOWCTL_FULL_DELAY = 2 /**< delay possible for extended period of time */ -} flowControl_t; - -/* filter operations */ -typedef enum { - FIOP_NOP = 0, /* do not use - No Operation */ - FIOP_CONTAINS = 1, /* contains string? */ - FIOP_ISEQUAL = 2, /* is (exactly) equal? */ - FIOP_STARTSWITH = 3, /* starts with a string? */ - FIOP_REGEX = 4, /* matches a (BRE) regular expression? */ - FIOP_EREREGEX = 5 /* matches a ERE regular expression? */ -} fiop_t; - - -/* multi-submit support. - * This is done via a simple data structure, which holds the number of elements - * as well as an array of to-be-submitted messages. - * rgerhards, 2009-06-16 - */ -typedef struct multi_submit_s multi_submit_t; -struct multi_submit_s { - short maxElem; /* maximum number of Elements */ - short nElem; /* current number of Elements, points to the next one FREE */ - msg_t **ppMsgs; -}; - #ifndef _PATH_CONSOLE #define _PATH_CONSOLE "/dev/console" diff --git a/runtime/typedefs.h b/runtime/typedefs.h new file mode 100644 index 00000000..2f504179 --- /dev/null +++ b/runtime/typedefs.h @@ -0,0 +1,147 @@ +/* This defines some types commonly used. Do NOT include any other + * rsyslog runtime file. + * + * Begun 2010-11-25 RGerhards + * + * Copyright (C) 2005-2008 by Rainer Gerhards and Adiscon GmbH + * + * This file is part of the rsyslog runtime library. + * + * The rsyslog runtime library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * The rsyslog runtime library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with the rsyslog runtime library. If not, see . + * + * A copy of the GPL can be found in the file "COPYING" in this distribution. + * A copy of the LGPL can be found in the file "COPYING.LESSER" in this distribution. + */ +#ifndef INCLUDED_TYPEDEFS_H +#define INCLUDED_TYPEDEFS_H + +/* some universal fixed size integer defines ... */ +typedef long long int64; +typedef long long unsigned uint64; +typedef int64 number_t; /* type to use for numbers - TODO: maybe an autoconf option? */ +typedef char intTiny; /* 0..127! */ +typedef unsigned char uintTiny; /* 0..255! */ + +/* define some base data types */ + +typedef unsigned char uchar;/* get rid of the unhandy "unsigned char" */ +typedef struct aUsrp_s aUsrp_t; +typedef struct thrdInfo thrdInfo_t; +typedef struct obj_s obj_t; +typedef struct ruleset_s ruleset_t; +typedef struct rule_s rule_t; +//typedef struct filed selector_t;/* TODO: this so far resides in syslogd.c, think about modularization */ +typedef struct NetAddr netAddr_t; +typedef struct netstrms_s netstrms_t; +typedef struct netstrm_s netstrm_t; +typedef struct nssel_s nssel_t; +typedef struct nspoll_s nspoll_t; +typedef enum nsdsel_waitOp_e nsdsel_waitOp_t; +typedef struct nsd_ptcp_s nsd_ptcp_t; +typedef struct nsd_gtls_s nsd_gtls_t; +typedef struct nsd_gsspi_s nsd_gsspi_t; +typedef struct nsd_nss_s nsd_nss_t; +typedef struct nsdsel_ptcp_s nsdsel_ptcp_t; +typedef struct nsdsel_gtls_s nsdsel_gtls_t; +typedef struct nsdpoll_ptcp_s nsdpoll_ptcp_t; +typedef struct wti_s wti_t; +typedef struct msg msg_t; +typedef struct queue_s qqueue_t; +typedef struct prop_s prop_t; +typedef struct interface_s interface_t; +typedef struct objInfo_s objInfo_t; +typedef enum rsRetVal_ rsRetVal; /**< friendly type for global return value */ +typedef rsRetVal (*errLogFunc_t)(uchar*); /* this is a trick to store a function ptr to a function returning a function ptr... */ +typedef struct permittedPeers_s permittedPeers_t; /* this should go away in the long term -- rgerhards, 2008-05-19 */ +typedef struct permittedPeerWildcard_s permittedPeerWildcard_t; /* this should go away in the long term -- rgerhards, 2008-05-19 */ +typedef struct tcpsrv_s tcpsrv_t; +typedef struct tcps_sess_s tcps_sess_t; +typedef struct strmsrv_s strmsrv_t; +typedef struct strms_sess_s strms_sess_t; +typedef struct vmstk_s vmstk_t; +typedef struct batch_obj_s batch_obj_t; +typedef struct batch_s batch_t; +typedef struct wtp_s wtp_t; +typedef struct modInfo_s modInfo_t; +typedef struct parser_s parser_t; +typedef struct parserList_s parserList_t; +typedef struct strgen_s strgen_t; +typedef struct strgenList_s strgenList_t; +typedef rsRetVal (*prsf_t)(struct vmstk_s*, int); /* pointer to a RainerScript function */ +typedef uint64 qDeqID; /* queue Dequeue order ID. 32 bits is considered dangerously few */ + +typedef struct tcpLstnPortList_s tcpLstnPortList_t; // TODO: rename? +typedef struct strmLstnPortList_s strmLstnPortList_t; // TODO: rename? + +/* under Solaris (actually only SPARC), we need to redefine some types + * to be void, so that we get void* pointers. Otherwise, we will see + * alignment errors. + */ +#ifdef OS_SOLARIS + typedef void * obj_t_ptr; + typedef void nsd_t; + typedef void nsdsel_t; + typedef void nsdpoll_t; +#else + typedef obj_t *obj_t_ptr; + typedef obj_t nsd_t; + typedef obj_t nsdsel_t; + typedef obj_t nsdpoll_t; +#endif + + +#ifdef __hpux +typedef unsigned int u_int32_t; /* TODO: is this correct? */ +typedef int socklen_t; +#endif + +typedef struct epoll_event epoll_event_t; + +typedef char sbool; /* (small bool) I intentionally use char, to keep it slim so that many fit into the CPU cache! */ + +/* settings for flow control + * TODO: is there a better place for them? -- rgerhards, 2008-03-14 + */ +typedef enum { + eFLOWCTL_NO_DELAY = 0, /**< UDP and other non-delayable sources */ + eFLOWCTL_LIGHT_DELAY = 1, /**< some light delay possible, but no extended period of time */ + eFLOWCTL_FULL_DELAY = 2 /**< delay possible for extended period of time */ +} flowControl_t; + +/* filter operations */ +typedef enum { + FIOP_NOP = 0, /* do not use - No Operation */ + FIOP_CONTAINS = 1, /* contains string? */ + FIOP_ISEQUAL = 2, /* is (exactly) equal? */ + FIOP_STARTSWITH = 3, /* starts with a string? */ + FIOP_REGEX = 4, /* matches a (BRE) regular expression? */ + FIOP_EREREGEX = 5 /* matches a ERE regular expression? */ +} fiop_t; + + +/* multi-submit support. + * This is done via a simple data structure, which holds the number of elements + * as well as an array of to-be-submitted messages. + * rgerhards, 2009-06-16 + */ +typedef struct multi_submit_s multi_submit_t; +struct multi_submit_s { + short maxElem; /* maximum number of Elements */ + short nElem; /* current number of Elements, points to the next one FREE */ + msg_t **ppMsgs; +}; + +#endif /* multi-include protection */ +/* vim:set ai: + */ diff --git a/tools/omfile.c b/tools/omfile.c index 57089cfd..78f2bf8e 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -98,7 +98,11 @@ static pthread_mutex_t mutClock; static inline uint64 getClockFileAccess(void) { - return ATOMIC_INC_AND_FETCH(&clockFileAccess, &mutClock); +#if HAVE_ATOMIC_BUILTINS_64BIT + return ATOMIC_INC_AND_FETCH_uint64(&clockFileAccess, &mutClock); +#else + return ATOMIC_INC_AND_FETCH_unsigned(&clockFileAccess, &mutClock); +#endif } -- cgit v1.2.3