From 3065bb8bfad190a830d75c02bdb2ad95faedd0e0 Mon Sep 17 00:00:00 2001
From: Rainer Gerhards <rgerhards@adiscon.com>
Date: Tue, 22 Oct 2019 13:17:38 +0200
Subject: [PATCH] omfile bugfix: file handle leak (bsc#1189737)

The stream class does not close re-opened file descriptors.
This lead to leaking file handles and ultimately to the inability
to open any files/sockets/etc as rsyslog ran out of handles.

The bug was depending on timing. This involed different OS
thread scheduler timing as well as workload. The bug was more
common under the following conditions:

- async writing of files
- dynafiles
- not commiting file data at end of transaction

However it could be triggerred under other conditions as well.

The refactoring done in 8.1908 increased the likelyhood of
experienceing this bug. But it was not a real regression, the new
code was valid, but changed the timing so that the race was more
likely.

Thanks to Michael Biebl for reporting this bug and helping to
analyze it.

closes https://github.com/rsyslog/rsyslog/issues/3885

(cherry picked from commit 19704e7095d8cdc946c2b346271ebe1e76f4cf0c)
---
 runtime/stream.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/runtime/stream.c b/runtime/stream.c
index 3e879c486..6195cef8a 100644
--- a/runtime/stream.c
+++ b/runtime/stream.c
@@ -408,6 +408,19 @@ strmWaitAsyncWriterDone(strm_t *pThis)
 	ENDfunc
 }
 
+/* stop the writer thread (we MUST be runnnig asynchronously when this method
+ * is called!). Note that the mutex must be locked! -- rgerhards, 2009-07-06
+ */
+static void
+stopWriter(strm_t *const pThis)
+{
+	pThis->bStopWriter = 1;
+	pthread_cond_signal(&pThis->notEmpty);
+	d_pthread_mutex_unlock(&pThis->mut);
+	pthread_join(pThis->writerThreadID, NULL);
+}
+
+
 
 /* close a strm file
  * Note that the bDeleteOnClose flag is honored. If it is set, the file will be
@@ -434,6 +447,9 @@ static rsRetVal strmCloseFile(strm_t *pThis)
 		if(pThis->iZipLevel) {
 			doZipFinish(pThis);
 		}
+		if(pThis->bAsyncWrite) {
+			stopWriter(pThis);
+		}
 	}
 
 	/* if we have a signature provider, we must make sure that the crypto
@@ -446,6 +462,8 @@ static rsRetVal strmCloseFile(strm_t *pThis)
 	 * against this. -- rgerhards, 2010-03-19
 	 */
 	if(pThis->fd != -1) {
+		DBGOPRINT((obj_t*) pThis, "file %d(%s) closing\n",
+			pThis->fd, getFileDebugName(pThis));
 		currOffs = lseek64(pThis->fd, 0, SEEK_CUR);
 		close(pThis->fd);
 		pThis->fd = -1;
@@ -1055,29 +1073,13 @@ finalize_it:
 	RETiRet;
 }
 
-
-/* stop the writer thread (we MUST be runnnig asynchronously when this method
- * is called!). Note that the mutex must be locked! -- rgerhards, 2009-07-06
- */
-static void
-stopWriter(strm_t *pThis)
-{
-	BEGINfunc
-	pThis->bStopWriter = 1;
-	pthread_cond_signal(&pThis->notEmpty);
-	d_pthread_mutex_unlock(&pThis->mut);
-	pthread_join(pThis->writerThreadID, NULL);
-	ENDfunc
-}
-
-
 /* destructor for the strm object */
 BEGINobjDestruct(strm) /* be sure to specify the object type also in END and CODESTART macros! */
 	int i;
 CODESTARTobjDestruct(strm)
 	/* we need to stop the ZIP writer */
 	if(pThis->bAsyncWrite)
-		/* Note: mutex will be unlocked in stopWriter! */
+		/* Note: mutex will be unlocked in strmCloseFile/stopWriter! */
 		d_pthread_mutex_lock(&pThis->mut);
 
 	/* strmClose() will handle read-only files as well as need to open
@@ -1086,7 +1088,6 @@ CODESTARTobjDestruct(strm)
 	strmCloseFile(pThis);
 
 	if(pThis->bAsyncWrite) {
-		stopWriter(pThis);
 		pthread_mutex_destroy(&pThis->mut);
 		pthread_cond_destroy(&pThis->notFull);
 		pthread_cond_destroy(&pThis->notEmpty);
@@ -1429,6 +1430,8 @@ asyncWriterThread(void *pPtr)
 	/* Not reached */	
 
 finalize_it:
+	DBGOPRINT((obj_t*) pThis, "file %d(%s) asyncWriterThread terminated\n",
+		pThis->fd, getFileDebugName(pThis));
 	ENDfunc
 	return NULL; /* to keep pthreads happy */
 }
@@ -1656,6 +1659,8 @@ strmFlush(strm_t *pThis)
 
 	ASSERT(pThis != NULL);
 
+	DBGOPRINT((obj_t*) pThis, "file %d strmFlush\n", pThis->fd);
+
 	if(pThis->bAsyncWrite)
 		d_pthread_mutex_lock(&pThis->mut);
 	CHKiRet(strmFlushInternal(pThis, 1));
-- 
2.31.1

