summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRainer Gerhards <rgerhards@adiscon.com>2009-01-27 22:41:14 +0100
committerRainer Gerhards <rgerhards@adiscon.com>2009-01-27 22:41:14 +0100
commit35673b12c42429786f6229ff9fcef7001a6b21ab (patch)
tree9f753817c379fe121e37fdefe3d39bef7c9714a6
parent20ff1ed403f05606b68c13e4d5c591c6b8706f86 (diff)
downloadrsyslog-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.c19
1 files changed, 7 insertions, 12 deletions
diff --git a/msg.c b/msg.c
index bd1e425e..107df1f9 100644
--- a/msg.c
+++ b/msg.c
@@ -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);
}