diff options
author | Rainer Gerhards <rgerhards@adiscon.com> | 2009-01-27 22:41:14 +0100 |
---|---|---|
committer | Rainer Gerhards <rgerhards@adiscon.com> | 2009-01-27 22:41:14 +0100 |
commit | 35673b12c42429786f6229ff9fcef7001a6b21ab (patch) | |
tree | 9f753817c379fe121e37fdefe3d39bef7c9714a6 | |
parent | 20ff1ed403f05606b68c13e4d5c591c6b8706f86 (diff) | |
download | rsyslog-35673b12c42429786f6229ff9fcef7001a6b21ab.tar.gz rsyslog-35673b12c42429786f6229ff9fcef7001a6b21ab.tar.xz rsyslog-35673b12c42429786f6229ff9fcef7001a6b21ab.zip |
bugfix: proper message locking on message destruct
It looks like a race was introduced by not locking the message mutex
in msgDestruct(). In theory, I thought, the decrement should be atomic,
but the whole operation may be reordered. Also it has potential for task
switches. If so, that would lead to a too-early destruction and thus
a potential double free - exactly what we have seen from time to time.
So I think this fix addresses the issue.
I have also removed anything that looks like atomic operations are supported
in this version - they are not. This was very late added, found to be
non-portable and pulled from that release.
-rw-r--r-- | msg.c | 19 |
1 files changed, 7 insertions, 12 deletions
@@ -272,11 +272,8 @@ BEGINobjDestruct(msg) /* be sure to specify the object type also in END and CODE int currRefCount; CODESTARTobjDestruct(msg) /* DEV Debugging only ! dbgprintf("msgDestruct\t0x%lx, Ref now: %d\n", (unsigned long)pM, pM->iRefCount - 1); */ -# ifdef DO_HAVE_ATOMICS - currRefCount = ATOMIC_DEC_AND_FETCH(pThis->iRefCount); -# else - currRefCount = --pThis->iRefCount; -# endif + MsgLock(pM); + currRefCount = --pThis->iRefCount; if(currRefCount == 0) { /* DEV Debugging Only! dbgprintf("msgDestruct\t0x%lx, RefCount now 0, doing DESTROY\n", (unsigned long)pThis); */ @@ -328,8 +325,10 @@ CODESTARTobjDestruct(msg) rsCStrDestruct(&pThis->pCSPROCID); if(pThis->pCSMSGID != NULL) rsCStrDestruct(&pThis->pCSMSGID); + MsgUnlock(pM); funcDeleteMutex(pThis); } else { + MsgUnlock(pM); pThis = NULL; /* tell framework not to destructing the object! */ } ENDobjDestruct(msg) @@ -472,13 +471,9 @@ finalize_it: msg_t *MsgAddRef(msg_t *pM) { assert(pM != NULL); -# ifdef DO_HAVE_ATOMICS - ATOMIC_INC(pM->iRefCount); -# else - MsgLock(pM); - pM->iRefCount++; - MsgUnlock(pM); -# endif + MsgLock(pM); + pM->iRefCount++; + MsgUnlock(pM); /* DEV debugging only! dbgprintf("MsgAddRef\t0x%x done, Ref now: %d\n", (int)pM, pM->iRefCount);*/ return(pM); } |