From 5b4e06fc28ef217e9ca26611e11afd974bdd1a4a Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 15 Jan 2010 17:01:10 +0100 Subject: bugfix: rsyslog hangs when writing to a named pipe which nobody was reading. Thanks to Michael Biebl for reporting this bug. --- ChangeLog | 6 +++++- runtime/stream.c | 13 ++++++++++--- runtime/stream.h | 3 ++- tools/omfile.c | 22 ++++++++++++++++------ 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index ad94566d..c58d5b78 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,8 @@ -- fixed a memory leak when sending messages in zip-compressed format +--------------------------------------------------------------------------- +Version 4.5.8 [v4-beta] (rgerhards), 2010-01-?? +- bugfix: rsyslog hang when writing to a named pipe which nobody was + reading. Thanks to Michael Biebl for reporting this bug. +- bugfix: memory leak when sending messages in zip-compressed format Thanks to Naoya Nakazawa for analyzing this issue and providing a patch. --------------------------------------------------------------------------- Version 4.5.7 [v4-beta] (rgerhards), 2009-11-18 diff --git a/runtime/stream.c b/runtime/stream.c index 2d1e9380..36f44003 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -209,13 +209,19 @@ doPhysOpen(strm_t *pThis) default:assert(0); break; } + if(pThis->sType == STREAMTYPE_NAMED_PIPE) { + DBGPRINTF("Note: stream '%s' is a named pipe, open with O_NONBLOCK\n", pThis->pszCurrFName); + iFlags |= O_NONBLOCK; + } pThis->fd = open((char*)pThis->pszCurrFName, iFlags, pThis->tOpenMode); DBGPRINTF("file '%s' opened as #%d with mode %d\n", pThis->pszCurrFName, pThis->fd, pThis->tOpenMode); if(pThis->fd == -1) { - int ierrnoSave = errno; - dbgoprint((obj_t*) pThis, "open error %d, file '%s'\n", errno, pThis->pszCurrFName); - if(ierrnoSave == ENOENT) + char errStr[1024]; + int err = errno; + rs_strerror_r(err, errStr, sizeof(errStr)); + dbgoprint((obj_t*) pThis, "open error %d, file '%s': %s\n", errno, pThis->pszCurrFName, errStr); + if(err == ENOENT) ABORT_FINALIZE(RS_RET_FILE_NOT_FOUND); else ABORT_FINALIZE(RS_RET_IO_ERROR); @@ -429,6 +435,7 @@ strmHandleEOF(strm_t *pThis) ISOBJ_TYPE_assert(pThis, strm); switch(pThis->sType) { case STREAMTYPE_FILE_SINGLE: + case STREAMTYPE_NAMED_PIPE: ABORT_FINALIZE(RS_RET_EOF); break; case STREAMTYPE_FILE_CIRCULAR: diff --git a/runtime/stream.h b/runtime/stream.h index 64ffb6e1..89175b0f 100644 --- a/runtime/stream.h +++ b/runtime/stream.h @@ -76,7 +76,8 @@ typedef enum { STREAMTYPE_FILE_SINGLE = 0, /**< read a single file */ STREAMTYPE_FILE_CIRCULAR = 1, /**< circular files */ - STREAMTYPE_FILE_MONITOR = 2 /**< monitor a (third-party) file */ + STREAMTYPE_FILE_MONITOR = 2, /**< monitor a (third-party) file */ + STREAMTYPE_NAMED_PIPE = 3 /**< file is a named pipe (so far, tested for output only) */ } strmType_t; typedef enum { /* when extending, do NOT change existing modes! */ diff --git a/tools/omfile.c b/tools/omfile.c index 209adc51..db49a05c 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -109,6 +109,7 @@ static uchar *pszTplName = NULL; /* name of the default template to use */ typedef struct _instanceData { uchar f_fname[MAXFNAME];/* file or template name (display only) */ strm_t *pStrm; /* our output stream */ + strmType_t strmType; /* stream type, used for named pipes */ char bDynamicName; /* 0 - static name, 1 - dynamic name (with properties) */ int fCreateMode; /* file creation mode for open() */ int fDirCreateMode; /* creation mode for mkdir() */ @@ -405,7 +406,7 @@ prepareFile(instanceData *pData, uchar *newFileName) CHKiRet(strm.SettOperationsMode(pData->pStrm, STREAMMODE_WRITE_APPEND)); CHKiRet(strm.SettOpenMode(pData->pStrm, fCreateMode)); CHKiRet(strm.SetbSync(pData->pStrm, pData->bSyncFile)); - CHKiRet(strm.SetsType(pData->pStrm, STREAMTYPE_FILE_SINGLE)); + CHKiRet(strm.SetsType(pData->pStrm, pData->strmType)); CHKiRet(strm.SetiSizeLimit(pData->pStrm, pData->iSizeLimit)); /* set the flush interval only if we actually use it - otherwise it will activate * async processing, which is a real performance waste if we do not do buffered @@ -576,8 +577,11 @@ writeFile(uchar **ppString, unsigned iMsgOpts, instanceData *pData) finalize_it: if(iRet != RS_RET_OK) { - /* in v5, we shall return different states for message-cause failur (but only there!) */ - iRet = RS_RET_SUSPENDED; + /* in v5, we shall return different states for message-caused failure (but only there!) */ + if(pData->strmType == STREAMTYPE_NAMED_PIPE) + iRet = RS_RET_DISABLE_ACTION; /* this is the traditional semantic -- rgerhards, 2010-01-15 */ + else + iRet = RS_RET_SUSPENDED; } RETiRet; } @@ -665,11 +669,17 @@ CODESTARTparseSelectorAct case '|': case '/': CODE_STD_STRING_REQUESTparseSelectorAct(1) - /* we now have the same semantics for files and pipes, but we need to skip over - * the pipe indicator traditionally seen in config files... + /* we now have *almost* the same semantics for files and pipes, but we still need + * to know we deal with a pipe, because we must do non-blocking opens in that case + * (to keep consistent with traditional semantics and prevent rsyslog from hanging). */ - if(*p == '|') + if(*p == '|') { ++p; + pData->strmType = STREAMTYPE_NAMED_PIPE; + } else { + pData->strmType = STREAMTYPE_FILE_SINGLE; + } + CHKiRet(cflineParseFileName(p, (uchar*) pData->f_fname, *ppOMSR, 0, OMSR_NO_RQD_TPL_OPTS, (pszTplName == NULL) ? (uchar*)"RSYSLOG_FileFormat" : pszTplName)); pData->bDynamicName = 0; -- cgit