bugfix: race condition in deferred name resolution

Note that this actually is a very small change, but I needed
to shuffle a lot of code around in order to make it compile
(due to required define order...).

1 files changed, 117 insertions, 115 deletions

diff --git a/runtime/msg.c b/runtime/msg.c
index e8e60963..d1e67aa2 100644
--- a/runtime/msg.c
+++ b/runtime/msg.c
@@ -287,6 +287,121 @@ static pthread_mutex_t mutTrimCtr;	 /* mutex to handle malloc trim */
 static int getAPPNAMELen(msg_t *pM, sbool bLockMutex);
 
 
+/* The following functions will support advanced output module
+ * multithreading, once this is implemented. Currently, we
+ * include them as hooks only. The idea is that we need to guard
+ * some msg objects data fields against concurrent access if
+ * we run on multiple threads. Please note that in any case this
+ * is not necessary for calls from INPUT modules, because they
+ * construct the message object and do this serially. Only when
+ * the message is in the processing queue, multiple threads may
+ * access a single object. Consequently, there are no guard functions
+ * for "set" methods, as these are called during input. Only "get"
+ * functions that modify important structures have them.
+ * rgerhards, 2007-07-20
+ * We now support locked and non-locked operations, depending on
+ * the configuration of rsyslog. To support this, we use function
+ * pointers. Initially, we start in non-locked mode. There, all
+ * locking operations call into dummy functions. When locking is
+ * enabled, the function pointers are changed to functions doing
+ * actual work. We also introduced another MsgPrepareEnqueue() function
+ * which initializes the locking structures, if needed. This is
+ * necessary because internal messages during config file startup
+ * processing are always created in non-locking mode. So we can
+ * not initialize locking structures during constructions. We now
+ * postpone this until when the message is fully constructed and
+ * enqueued. Then we know the status of locking. This has a nice
+ * side effect, and that is that during the initial creation of
+ * the Msg object no locking needs to be done, which results in better
+ * performance. -- rgerhards, 2008-01-05
+ */
+static void (*funcLock)(msg_t *pMsg);
+static void (*funcUnlock)(msg_t *pMsg);
+static void (*funcDeleteMutex)(msg_t *pMsg);
+void (*funcMsgPrepareEnqueue)(msg_t *pMsg);
+#if 1 /* This is a debug aid */
+#define MsgLock(pMsg) 	funcLock(pMsg)
+#define MsgUnlock(pMsg) funcUnlock(pMsg)
+#else
+#define MsgLock(pMsg) 	{dbgprintf("MsgLock line %d\n - ", __LINE__); funcLock(pMsg);; }
+#define MsgUnlock(pMsg) {dbgprintf("MsgUnlock line %d - ", __LINE__); funcUnlock(pMsg); }
+#endif
+
+/* the next function is a dummy to be used by the looking functions
+ * when the class is not yet running in an environment where locking
+ * is necessary. Please note that the need to lock can (and will) change
+ * during a single run. Typically, this is depending on the operation mode
+ * of the message queues (which is operator-configurable). -- rgerhards, 2008-01-05
+ */
+static void MsgLockingDummy(msg_t __attribute__((unused)) *pMsg)
+{
+	/* empty be design */
+}
+
+
+/* The following function prepares a message for enqueue into the queue. This is
+ * where a message may be accessed by multiple threads. This implementation here
+ * is the version for multiple concurrent acces. It initializes the locking
+ * structures.
+ * TODO: change to an iRet interface! -- rgerhards, 2008-07-14
+ */
+static void MsgPrepareEnqueueLockingCase(msg_t *pThis)
+{
+	BEGINfunc
+	assert(pThis != NULL);
+	pthread_mutex_init(&pThis->mut, NULL);
+	pThis->bDoLock = 1;
+	ENDfunc
+}
+
+
+/* ... and now the locking and unlocking implementations: */
+static void MsgLockLockingCase(msg_t *pThis)
+{
+	/* DEV debug only! dbgprintf("MsgLock(0x%lx)\n", (unsigned long) pThis); */
+	assert(pThis != NULL);
+	if(pThis->bDoLock == 1) /* TODO: this is a testing hack, we should find a way with better performance! -- rgerhards, 2009-01-27 */
+		pthread_mutex_lock(&pThis->mut);
+}
+
+static void MsgUnlockLockingCase(msg_t *pThis)
+{
+	/* DEV debug only! dbgprintf("MsgUnlock(0x%lx)\n", (unsigned long) pThis); */
+	assert(pThis != NULL);
+	if(pThis->bDoLock == 1) /* TODO: this is a testing hack, we should find a way with better performance! -- rgerhards, 2009-01-27 */
+		pthread_mutex_unlock(&pThis->mut);
+}
+
+/* delete the mutex object on message destruction (locking case)
+ */
+static void MsgDeleteMutexLockingCase(msg_t *pThis)
+{
+	assert(pThis != NULL);
+	pthread_mutex_destroy(&pThis->mut);
+}
+
+/* enable multiple concurrent access on the message object
+ * This works on a class-wide basis and can bot be undone.
+ * That is, if it is once enabled, it can not be disabled during
+ * the same run. When this function is called, no other thread
+ * must manipulate message objects. Then we would have race conditions,
+ * but guarding against this is counter-productive because it
+ * would cost additional time. Plus, it would be a programming error.
+ * rgerhards, 2008-01-05
+ */
+rsRetVal MsgEnableThreadSafety(void)
+{
+	DEFiRet;
+	funcLock = MsgLockLockingCase;
+	funcUnlock = MsgUnlockLockingCase;
+	funcMsgPrepareEnqueue = MsgPrepareEnqueueLockingCase;
+	funcDeleteMutex = MsgDeleteMutexLockingCase;
+	RETiRet;
+}
+
+/* end locking functions */
+
+
 static inline int getProtocolVersion(msg_t *pM)
 {
 	return(pM->iProtocolVersion);
@@ -306,6 +421,7 @@ resolveDNS(msg_t *pMsg) {
 	uchar fromHostFQDN[NI_MAXHOST];
 	DEFiRet;
 
+	MsgLock(pMsg);
 	CHKiRet(objUse(net, CORE_COMPONENT));
 	if(pMsg->msgFlags & NEEDS_DNSRESOL) {
 		localRet = net.cvthname(pMsg->rcvFrom.pfrominet, fromHost, fromHostFQDN, fromHostIP);
@@ -315,6 +431,7 @@ resolveDNS(msg_t *pMsg) {
 		}
 	}
 finalize_it:
+	MsgUnlock(pMsg);
 	if(iRet != RS_RET_OK) {
 		/* best we can do: remove property */
 		MsgSetRcvFromStr(pMsg, UCHAR_CONSTANT(""), 0, &propFromHost);
@@ -535,121 +652,6 @@ uchar *propIDToName(propid_t propID)
 }
 
 
-/* The following functions will support advanced output module
- * multithreading, once this is implemented. Currently, we
- * include them as hooks only. The idea is that we need to guard
- * some msg objects data fields against concurrent access if
- * we run on multiple threads. Please note that in any case this
- * is not necessary for calls from INPUT modules, because they
- * construct the message object and do this serially. Only when
- * the message is in the processing queue, multiple threads may
- * access a single object. Consequently, there are no guard functions
- * for "set" methods, as these are called during input. Only "get"
- * functions that modify important structures have them.
- * rgerhards, 2007-07-20
- * We now support locked and non-locked operations, depending on
- * the configuration of rsyslog. To support this, we use function
- * pointers. Initially, we start in non-locked mode. There, all
- * locking operations call into dummy functions. When locking is
- * enabled, the function pointers are changed to functions doing
- * actual work. We also introduced another MsgPrepareEnqueue() function
- * which initializes the locking structures, if needed. This is
- * necessary because internal messages during config file startup
- * processing are always created in non-locking mode. So we can
- * not initialize locking structures during constructions. We now
- * postpone this until when the message is fully constructed and
- * enqueued. Then we know the status of locking. This has a nice
- * side effect, and that is that during the initial creation of
- * the Msg object no locking needs to be done, which results in better
- * performance. -- rgerhards, 2008-01-05
- */
-static void (*funcLock)(msg_t *pMsg);
-static void (*funcUnlock)(msg_t *pMsg);
-static void (*funcDeleteMutex)(msg_t *pMsg);
-void (*funcMsgPrepareEnqueue)(msg_t *pMsg);
-#if 1 /* This is a debug aid */
-#define MsgLock(pMsg) 	funcLock(pMsg)
-#define MsgUnlock(pMsg) funcUnlock(pMsg)
-#else
-#define MsgLock(pMsg) 	{dbgprintf("MsgLock line %d\n - ", __LINE__); funcLock(pMsg);; }
-#define MsgUnlock(pMsg) {dbgprintf("MsgUnlock line %d - ", __LINE__); funcUnlock(pMsg); }
-#endif
-
-/* the next function is a dummy to be used by the looking functions
- * when the class is not yet running in an environment where locking
- * is necessary. Please note that the need to lock can (and will) change
- * during a single run. Typically, this is depending on the operation mode
- * of the message queues (which is operator-configurable). -- rgerhards, 2008-01-05
- */
-static void MsgLockingDummy(msg_t __attribute__((unused)) *pMsg)
-{
-	/* empty be design */
-}
-
-
-/* The following function prepares a message for enqueue into the queue. This is
- * where a message may be accessed by multiple threads. This implementation here
- * is the version for multiple concurrent acces. It initializes the locking
- * structures.
- * TODO: change to an iRet interface! -- rgerhards, 2008-07-14
- */
-static void MsgPrepareEnqueueLockingCase(msg_t *pThis)
-{
-	BEGINfunc
-	assert(pThis != NULL);
-	pthread_mutex_init(&pThis->mut, NULL);
-	pThis->bDoLock = 1;
-	ENDfunc
-}
-
-
-/* ... and now the locking and unlocking implementations: */
-static void MsgLockLockingCase(msg_t *pThis)
-{
-	/* DEV debug only! dbgprintf("MsgLock(0x%lx)\n", (unsigned long) pThis); */
-	assert(pThis != NULL);
-	if(pThis->bDoLock == 1) /* TODO: this is a testing hack, we should find a way with better performance! -- rgerhards, 2009-01-27 */
-		pthread_mutex_lock(&pThis->mut);
-}
-
-static void MsgUnlockLockingCase(msg_t *pThis)
-{
-	/* DEV debug only! dbgprintf("MsgUnlock(0x%lx)\n", (unsigned long) pThis); */
-	assert(pThis != NULL);
-	if(pThis->bDoLock == 1) /* TODO: this is a testing hack, we should find a way with better performance! -- rgerhards, 2009-01-27 */
-		pthread_mutex_unlock(&pThis->mut);
-}
-
-/* delete the mutex object on message destruction (locking case)
- */
-static void MsgDeleteMutexLockingCase(msg_t *pThis)
-{
-	assert(pThis != NULL);
-	pthread_mutex_destroy(&pThis->mut);
-}
-
-/* enable multiple concurrent access on the message object
- * This works on a class-wide basis and can bot be undone.
- * That is, if it is once enabled, it can not be disabled during
- * the same run. When this function is called, no other thread
- * must manipulate message objects. Then we would have race conditions,
- * but guarding against this is counter-productive because it
- * would cost additional time. Plus, it would be a programming error.
- * rgerhards, 2008-01-05
- */
-rsRetVal MsgEnableThreadSafety(void)
-{
-	DEFiRet;
-	funcLock = MsgLockLockingCase;
-	funcUnlock = MsgUnlockLockingCase;
-	funcMsgPrepareEnqueue = MsgPrepareEnqueueLockingCase;
-	funcDeleteMutex = MsgDeleteMutexLockingCase;
-	RETiRet;
-}
-
-/* end locking functions */
-
-
 /* This is common code for all Constructors. It is defined in an
  * inline'able function so that we can save a function call in the
  * actual constructors (otherwise, the msgConstruct would need