bugfix: rsyslog hangs when writing to a named pipe which nobody was reading.

Thanks to Michael Biebl for reporting this bug.

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;