summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRainer Gerhards <rgerhards@adiscon.com>2011-06-21 12:35:14 +0200
committerRainer Gerhards <rgerhards@adiscon.com>2011-06-21 12:35:14 +0200
commit86225089f2d0e82deb368e1688464e8ba84d24cf (patch)
tree2ea4ac8481149d9537644246aa0a6d1f359ac03c
parentde33ec026e505658d613b372ea9d32656ebb86d8 (diff)
downloadrsyslog-86225089f2d0e82deb368e1688464e8ba84d24cf.tar.gz
rsyslog-86225089f2d0e82deb368e1688464e8ba84d24cf.tar.xz
rsyslog-86225089f2d0e82deb368e1688464e8ba84d24cf.zip
bugfix: mutex was invalidly left unlocked during action processingv5.8.2
At least one case where this can occur is during thread shutdown, which may be initiated by lower activity. In most cases, this is quite unlikely to happen. However, if it does, data structures may be corrupted which could lead to fatal failure and segfault. I detected this via a testbench test, not a user report. But I assume that some users may have had unreproducable aborts that were cause by this bug.
-rw-r--r--ChangeLog7
-rw-r--r--runtime/queue.c12
-rwxr-xr-xtests/diag.sh2
3 files changed, 16 insertions, 5 deletions
diff --git a/ChangeLog b/ChangeLog
index 9cef03e6..e438ac61 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,13 @@ Version 5.8.2 [V5-stable] (rgerhards), 2011-06-21
- bugfix: problems in failover action handling
closes: http://bugzilla.adiscon.com/show_bug.cgi?id=270
closes: http://bugzilla.adiscon.com/show_bug.cgi?id=254
+- bugfix: mutex was invalidly left unlocked during action processing
+ At least one case where this can occur is during thread shutdown, which
+ may be initiated by lower activity. In most cases, this is quite
+ unlikely to happen. However, if it does, data structures may be
+ corrupted which could lead to fatal failure and segfault. I detected
+ this via a testbench test, not a user report. But I assume that some
+ users may have had unreproducable aborts that were cause by this bug.
- bugfix: memory leak in imtcp & subsystems under some circumstances
This leak is tied to error conditions which lead to incorrect cleanup
of some data structures. [backport from v6]
diff --git a/runtime/queue.c b/runtime/queue.c
index 88e01a7a..00eb76c7 100644
--- a/runtime/queue.c
+++ b/runtime/queue.c
@@ -1678,6 +1678,7 @@ static rsRetVal
ConsumerReg(qqueue_t *pThis, wti_t *pWti)
{
int iCancelStateSave;
+ int bNeedReLock = 0; /**< do we need to lock the mutex again? */
DEFiRet;
ISOBJ_TYPE_assert(pThis, qqueue);
@@ -1687,6 +1688,7 @@ ConsumerReg(qqueue_t *pThis, wti_t *pWti)
/* we now have a non-idle batch of work, so we can release the queue mutex and process it */
d_pthread_mutex_unlock(pThis->mut);
+ bNeedReLock = 1;
/* at this spot, we may be cancelled */
pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &iCancelStateSave);
@@ -1706,12 +1708,14 @@ ConsumerReg(qqueue_t *pThis, wti_t *pWti)
/* but now cancellation is no longer permitted */
pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave);
- /* now we are done, but need to re-aquire the mutex */
- d_pthread_mutex_lock(pThis->mut);
-
finalize_it:
- dbgprintf("regular consumer finished, iret=%d, szlog %d sz phys %d\n", iRet,
+ DBGPRINTF("regular consumer finished, iret=%d, szlog %d sz phys %d\n", iRet,
getLogicalQueueSize(pThis), getPhysicalQueueSize(pThis));
+
+ /* now we are done, but potentially need to re-aquire the mutex */
+ if(bNeedReLock)
+ d_pthread_mutex_lock(pThis->mut);
+
RETiRet;
}
diff --git a/tests/diag.sh b/tests/diag.sh
index 7651b1de..617118ba 100755
--- a/tests/diag.sh
+++ b/tests/diag.sh
@@ -10,7 +10,7 @@
#valgrind="valgrind --tool=helgrind --log-fd=1"
#valgrind="valgrind --tool=exp-ptrcheck --log-fd=1"
#set -o xtrace
-#export RSYSLOG_DEBUG="debug nologfuncflow nostdout"
+#export RSYSLOG_DEBUG="debug nologfuncflow noprintmutexaction stdout"
#export RSYSLOG_DEBUGLOG="log"
case $1 in
'init') $srcdir/killrsyslog.sh # kill rsyslogd if it runs for some reason