diff options
| author | Rainer Gerhards <rgerhards@adiscon.com> | 2008-09-18 12:19:33 +0200 |
|---|---|---|
| committer | Rainer Gerhards <rgerhards@adiscon.com> | 2008-09-18 12:19:33 +0200 |
| commit | 4c96ebdcfe075e80810b01257cf21ea1c9b3ec0e (patch) | |
| tree | b6768398d8d55c04e045b5213e11b952484025e8 /runtime | |
| parent | 988989e49ef8639123c83383ba256c4e67679c8d (diff) | |
| download | rsyslog-4c96ebdcfe075e80810b01257cf21ea1c9b3ec0e.tar.gz rsyslog-4c96ebdcfe075e80810b01257cf21ea1c9b3ec0e.tar.xz rsyslog-4c96ebdcfe075e80810b01257cf21ea1c9b3ec0e.zip | |
bugfix: potential race condition when adding messages to queue
There was a wrong order of mutex lock operations. It is hard to
believe that really caused problems, but in theory it could and with
threading we often see that theory becomes practice if something is only
used long enough on a fast enough machine with enough CPUs ;)
Diffstat (limited to 'runtime')
| -rw-r--r-- | runtime/atomic.h | 8 | ||||
| -rw-r--r-- | runtime/debug.h | 3 | ||||
| -rw-r--r-- | runtime/queue.c | 12 |
3 files changed, 14 insertions, 9 deletions
diff --git a/runtime/atomic.h b/runtime/atomic.h index d6811628..d15f78ee 100644 --- a/runtime/atomic.h +++ b/runtime/atomic.h @@ -41,11 +41,15 @@ * They simply came in too late. -- rgerhards, 2008-04-02 */ #ifdef HAVE_ATOMIC_BUILTINS -# define ATOMIC_INC(data) ((void) __sync_fetch_and_add(&data, 1)) -# define ATOMIC_DEC_AND_FETCH(data) __sync_sub_and_fetch(&data, 1) +# define ATOMIC_INC(data) ((void) __sync_fetch_and_add(&(data), 1)) +# define ATOMIC_DEC_AND_FETCH(data) __sync_sub_and_fetch(&(data), 1) +# define ATOMIC_FETCH_32BIT(data) ((unsigned) __sync_fetch_and_and(&(data), 0xffffffff)) +# define ATOMIC_STORE_1_TO_32BIT(data) __sync_lock_test_and_set(&(data), 1) #else # warning "atomic builtins not available, using nul operations" # define ATOMIC_INC(data) (++(data)) +# define ATOMIC_FETCH_32BIT(data) (data) +# define ATOMIC_STORE_1_TO_32BIT(data) (data) = 1 #endif #endif /* #ifndef INCLUDED_ATOMIC_H */ diff --git a/runtime/debug.h b/runtime/debug.h index 214b7c05..d9d576b5 100644 --- a/runtime/debug.h +++ b/runtime/debug.h @@ -130,7 +130,8 @@ void dbgPrintAllDebugInfo(void); /* debug aides */ -#ifdef RTINST +//#ifdef RTINST +#if 0 // temporarily removed for helgrind #define d_pthread_mutex_lock(x) dbgMutexLock(x, pdbgFuncDB, __LINE__, dbgCALLStaCK_POP_POINT ) #define d_pthread_mutex_unlock(x) dbgMutexUnlock(x, pdbgFuncDB, __LINE__, dbgCALLStaCK_POP_POINT ) #define d_pthread_cond_wait(cond, mut) dbgCondWait(cond, mut, pdbgFuncDB, __LINE__, dbgCALLStaCK_POP_POINT ) diff --git a/runtime/queue.c b/runtime/queue.c index 7e7d4152..c0a37019 100644 --- a/runtime/queue.c +++ b/runtime/queue.c @@ -2171,17 +2171,17 @@ queueEnqObj(queue_t *pThis, flowControl_t flowCtlType, void *pUsr) finalize_it: if(pThis->qType != QUEUETYPE_DIRECT) { - d_pthread_mutex_unlock(pThis->mut); + /* make sure at least one worker is running. */ + if(pThis->qType != QUEUETYPE_DIRECT) { + queueAdviseMaxWorkers(pThis); + } + /* and release the mutex */ i = pthread_cond_signal(&pThis->notEmpty); + d_pthread_mutex_unlock(pThis->mut); dbgoprint((obj_t*) pThis, "EnqueueMsg signaled condition (%d)\n", i); pthread_setcancelstate(iCancelStateSave, NULL); } - /* make sure at least one worker is running. */ - if(pThis->qType != QUEUETYPE_DIRECT) { - queueAdviseMaxWorkers(pThis); - } - RETiRet; } |