bugfix: imfile could cause a segfault upon rsyslogd HUP and termination

Thanks to lperr for an excellent bug report that helped detect this
problem.

4 files changed, 19 insertions, 7 deletions

diff --git a/ChangeLog b/ChangeLog
index 8933f88a..ff45137b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 ---------------------------------------------------------------------------
 Version 3.18.3 (rgerhards), 2008-08-??
+- bugfix: imfile could cause a segfault upon rsyslogd HUP and termination
+  Thanks to lperr for an excellent bug report that helped detect this
+  problem.
 - enhanced ommysql to support custom port to connect to server
   Port can be set via new $ActionOmmysqlServerPort config directive
   Note: this was a very minor change and thus deemed appropriate to be
diff --git a/configure.ac b/configure.ac
index caffe01e..b5427ab9 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.18.3-Test3],[rsyslog@lists.adiscon.com])
+AC_INIT([rsyslog],[3.18.3-Test4],[rsyslog@lists.adiscon.com])
 AM_INIT_AUTOMAKE
 AC_CONFIG_SRCDIR([syslogd.c])
 AC_CONFIG_HEADERS([config.h])
diff --git a/plugins/imfile/imfile.c b/plugins/imfile/imfile.c
index 75e54f04..6b7f516e 100644
--- a/plugins/imfile/imfile.c
+++ b/plugins/imfile/imfile.c
@@ -182,8 +182,9 @@ static void pollFileCancelCleanup(void *pArg)
 /* poll a file, need to check file rollover etc. open file if not open */
 static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData)
 {
-	DEFiRet;
 	cstr_t *pCStr = NULL;
+	int bMustPopCleanup = 0;
+	DEFiRet;
 
 	ASSERT(pbHadFileData != NULL);
 
@@ -192,6 +193,8 @@ static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData)
 	}
 
 	pthread_cleanup_push(pollFileCancelCleanup, &pCStr);
+	bMustPopCleanup = 1;
+
 	/* loop below will be exited when strmReadLine() returns EOF */
 	while(1) {
 		CHKiRet(strmReadLine(pThis->pStrm, &pCStr));
@@ -199,9 +202,11 @@ static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData)
 		CHKiRet(enqLine(pThis, pCStr)); /* process line */
 		rsCStrDestruct(&pCStr); /* discard string (must be done by us!) */
 	}
-	pthread_cleanup_pop(0);
 
 finalize_it:
+	if(bMustPopCleanup)
+		pthread_cleanup_pop(0);
+
 	if(pCStr != NULL) {
 		rsCStrDestruct(&pCStr);
 	}
diff --git a/threads.c b/threads.c
index e32ff0d9..03f19ff9 100644
--- a/threads.c
+++ b/threads.c
@@ -46,6 +46,7 @@ static linkedList_t llThrds;
  */
 static rsRetVal thrdConstruct(thrdInfo_t **ppThis)
 {
+	DEFiRet;
 	thrdInfo_t *pThis;
 
 	assert(ppThis != NULL);
@@ -60,7 +61,7 @@ static rsRetVal thrdConstruct(thrdInfo_t **ppThis)
 	pthread_mutex_init (pThis->mutTermOK, NULL);
 
 	*ppThis = pThis;
-	return RS_RET_OK;
+	RETiRet;
 }
 
 
@@ -70,6 +71,7 @@ static rsRetVal thrdConstruct(thrdInfo_t **ppThis)
  */
 static rsRetVal thrdDestruct(thrdInfo_t *pThis)
 {
+	DEFiRet;
 	assert(pThis != NULL);
 
 	if(pThis->bIsActive == 1) {
@@ -78,7 +80,7 @@ static rsRetVal thrdDestruct(thrdInfo_t *pThis)
 	free(pThis->mutTermOK);
 	free(pThis);
 
-	return RS_RET_OK;
+	RETiRet;
 }
 
 
@@ -86,6 +88,7 @@ static rsRetVal thrdDestruct(thrdInfo_t *pThis)
  */
 rsRetVal thrdTerminate(thrdInfo_t *pThis)
 {
+	DEFiRet;
 	assert(pThis != NULL);
 	
 	pthread_cancel(pThis->thrdID);
@@ -96,7 +99,7 @@ rsRetVal thrdTerminate(thrdInfo_t *pThis)
 	if(pThis->pAfterRun != NULL)
 		pThis->pAfterRun(pThis);
 	
-	return RS_RET_OK;
+	RETiRet;
 }
 
 
@@ -104,8 +107,9 @@ rsRetVal thrdTerminate(thrdInfo_t *pThis)
  */
 rsRetVal thrdTerminateAll(void)
 {
+	DEFiRet;
 	llDestroy(&llThrds);
-	return RS_RET_OK;
+	RETiRet;
 }