bugfix: inconsistent use of mutex/atomic operations could cause segfault
authorRainer Gerhards <rgerhards@adiscon.com>
Fri, 30 Jan 2009 12:49:41 +0000 (13:49 +0100)
committerRainer Gerhards <rgerhards@adiscon.com>
Fri, 30 Jan 2009 12:49:41 +0000 (13:49 +0100)
details are too many, for full analysis see blog post at:
http://blog.gerhards.net/2009/01/rsyslog-data-race-analysis.html

ChangeLog
runtime/atomic.h
runtime/msg.c

index d9a63d9..99e50b8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 ---------------------------------------------------------------------------
 Version 3.21.10 [BETA] (rgerhards), 2008-12-??
+- bugfix: inconsistent use of mutex/atomic operations could cause segfault
+  details are too many, for full analysis see blog post at:
+  http://blog.gerhards.net/2009/01/rsyslog-data-race-analysis.html
 - the string "Do Die" was accidently emited upon exit in non-debug mode
   This has now been corrected. Thanks to varmojfekoj for the patch.
 - some legacy options were not correctly processed.
index 2dbe7f5..2c20e0c 100644 (file)
@@ -46,7 +46,7 @@
 #      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"
+#      warning "atomic builtins not available, using nul operations - rsyslogd will probably be racy!"
 #      define ATOMIC_INC(data) (++(data))
 #      define ATOMIC_DEC_AND_FETCH(data) (--(data))
 #      define ATOMIC_FETCH_32BIT(data) (data)
index 3073fc5..038e002 100644 (file)
@@ -281,14 +281,13 @@ finalize_it:
 BEGINobjDestruct(msg) /* be sure to specify the object type also in END and CODESTART macros! */
        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
+       /* DEV Debugging only ! dbgprintf("msgDestruct\t0x%lx, Ref now: %d\n", (unsigned long)pThis, pThis->iRefCount - 1); */
+#      ifdef HAVE_ATOMIC_BUILTINS
+               currRefCount = ATOMIC_DEC_AND_FETCH(pThis->iRefCount);
+#      else
                MsgLock(pThis);
                currRefCount = --pThis->iRefCount;
-//#    endif
-// we need a mutex, because we may be suspended after getting the refcount but before
+#      endif
        if(currRefCount == 0)
        {
                /* DEV Debugging Only! dbgprintf("msgDestruct\t0x%lx, RefCount now 0, doing DESTROY\n", (unsigned long)pThis); */
@@ -348,7 +347,9 @@ CODESTARTobjDestruct(msg)
                        rsCStrDestruct(&pThis->pCSPROCID);
                if(pThis->pCSMSGID != NULL)
                        rsCStrDestruct(&pThis->pCSMSGID);
+#      ifndef HAVE_ATOMIC_BUILTINS
                MsgUnlock(pThis);
+#      endif
                funcDeleteMutex(pThis);
        } else {
                MsgUnlock(pThis);