[PATCH 0/1]: afmongodb: queue & wakeup race condition
There is a way to deadlock the mongodb destination, and I don't entirely understand the scenario, and thus, I can't properly explain it, either. Perhaps I'd need more coffee... but anyway! Following this mail, I'll send a patch that fixes the issue for me (I separately sent the same patch to the original reporter of the problem, too), but I'm not entirely happy with it. Nevertheless, I still find it better to send it to the list, where it can - perhaps - receive more testing than I was able to give it.
The problem was found and analysed by Whille, the patch is based on his observations. The patch corrects a situation where - due to a race condition - the worker thread could end up in a permanent sleeping state. Reported-by: whille <whille@163.com> Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- modules/afmongodb/afmongodb.c | 18 ++++++------------ 1 files changed, 6 insertions(+), 12 deletions(-) diff --git a/modules/afmongodb/afmongodb.c b/modules/afmongodb/afmongodb.c index 42c140e..47b6c9d 100644 --- a/modules/afmongodb/afmongodb.c +++ b/modules/afmongodb/afmongodb.c @@ -515,27 +515,21 @@ static void afmongodb_dd_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options, gpointer user_data) { MongoDBDestDriver *self = (MongoDBDestDriver *)s; - gboolean queue_was_empty; LogPathOptions local_options; if (!path_options->flow_control_requested) path_options = log_msg_break_ack(msg, path_options, &local_options); - g_mutex_lock(self->queue_mutex); self->last_msg_stamp = cached_g_current_time_sec (); - queue_was_empty = log_queue_get_length(self->queue) == 0; - g_mutex_unlock(self->queue_mutex); - - log_queue_push_tail(self->queue, msg, path_options); g_mutex_lock(self->suspend_mutex); - if (queue_was_empty && !self->writer_thread_suspended) - { - g_mutex_lock(self->queue_mutex); - log_queue_set_parallel_push(self->queue, 1, afmongodb_dd_queue_notify, self, NULL); - g_mutex_unlock(self->queue_mutex); - } + g_mutex_lock(self->queue_mutex); + if (!self->writer_thread_suspended) + log_queue_set_parallel_push(self->queue, 1, afmongodb_dd_queue_notify, + self, NULL); + g_mutex_unlock(self->queue_mutex); g_mutex_unlock(self->suspend_mutex); + log_queue_push_tail(self->queue, msg, path_options); } /* -- 1.7.9
We really have to nail the blocking log queue stuff. We have races here and there and I'm not really confident by this patch either. I've applied it anyway, but the priority of the proper solution is on the top of my syslog-ng related todo list now. On Fri, 2012-03-23 at 16:24 +0100, Gergely Nagy wrote:
The problem was found and analysed by Whille, the patch is based on his observations.
The patch corrects a situation where - due to a race condition - the worker thread could end up in a permanent sleeping state.
Reported-by: whille <whille@163.com> Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- modules/afmongodb/afmongodb.c | 18 ++++++- -- Bazsi
participants (2)
-
Balazs Scheidler
-
Gergely Nagy