[syslog-ng] [PATCH] afmongodb: Fix a queue notify<->wakeup deadlock

Gergely Nagy algernon at balabit.hu
Fri May 18 15:17:31 CEST 2012


As it turns out, there's a race condition between
afmongodb_dd_queue_notify() and the worker thread: one is trying to
lock queue_mutex to send a wakeup signal and clear the notification
callback, and the other is using g_cond_wait() with the same
queue_mutex locked around it.

There appears to be a race between the two threads, and it can happen
that both are trying to g_mutex_lock() the same mutex at the same
time.

To work around this problem, the patch below removes the queue_mutex
locking from dd_queue_notify(): it's not required anyway. But it also
moves the log_queue_reset_parallel_push() from the queue notify
function to the other thread, where we're already holding the mutex
after the g_cond_wait() anyway. (And do something similar in the
suspend wakeup case too, where locking queue_mutex is also safe).

This gets rid of the deadlock, and the code becomes easier to follow
aswell.

Reported-by: Eun Kyung <ekyung01 at googlemail.com>
Signed-off-by: Gergely Nagy <algernon at balabit.hu>
---
 modules/afmongodb/afmongodb.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/modules/afmongodb/afmongodb.c b/modules/afmongodb/afmongodb.c
index 5af34c9..7a1b6e2 100644
--- a/modules/afmongodb/afmongodb.c
+++ b/modules/afmongodb/afmongodb.c
@@ -388,6 +388,9 @@ afmongodb_worker_thread (gpointer arg)
 			    self->suspend_mutex,
 			    &self->writer_thread_suspend_target);
 	  self->writer_thread_suspended = FALSE;
+	  g_mutex_lock(self->queue_mutex);
+	  log_queue_reset_parallel_push(self->queue);
+	  g_mutex_unlock(self->queue_mutex);
 	  g_mutex_unlock(self->suspend_mutex);
 	}
       else
@@ -398,6 +401,7 @@ afmongodb_worker_thread (gpointer arg)
 	  if (log_queue_get_length(self->queue) == 0)
 	    {
 	      g_cond_wait(self->writer_thread_wakeup_cond, self->queue_mutex);
+	      log_queue_reset_parallel_push(self->queue);
 	    }
 	  g_mutex_unlock(self->queue_mutex);
 	}
@@ -560,10 +564,7 @@ afmongodb_dd_queue_notify(gpointer user_data)
 {
   MongoDBDestDriver *self = (MongoDBDestDriver *)user_data;
 
-  g_mutex_lock(self->queue_mutex);
   g_cond_signal(self->writer_thread_wakeup_cond);
-  log_queue_reset_parallel_push(self->queue);
-  g_mutex_unlock(self->queue_mutex);
 }
 
 static void
-- 
1.7.9




More information about the syslog-ng mailing list