diff options
author | Rainer Gerhards <rgerhards@adiscon.com> | 2011-06-21 12:35:14 +0200 |
---|---|---|
committer | Rainer Gerhards <rgerhards@adiscon.com> | 2011-06-21 12:35:14 +0200 |
commit | 86225089f2d0e82deb368e1688464e8ba84d24cf (patch) | |
tree | 2ea4ac8481149d9537644246aa0a6d1f359ac03c | |
parent | de33ec026e505658d613b372ea9d32656ebb86d8 (diff) | |
download | rsyslog-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-- | ChangeLog | 7 | ||||
-rw-r--r-- | runtime/queue.c | 12 | ||||
-rwxr-xr-x | tests/diag.sh | 2 |
3 files changed, 16 insertions, 5 deletions
@@ -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 |