Blob Blame History Raw
From 9b237463a8f5c75cff66364d07278217f8e4e586 Mon Sep 17 00:00:00 2001
From: Anand Avati <avati@redhat.com>
Date: Fri, 6 Dec 2013 17:31:57 -0800
Subject: [PATCH 243/244] timer: fix race between gf_timer_call_cancel() and gf_timer_proc()

        Backport of http://review.gluster.org/6459

Change-Id: Ie264d3d591352e4a8ddaa90ae2174d9c552396f1
BUG: 1242423
Signed-off-by: Anand Avati <avati@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/53060
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
---
 libglusterfs/src/timer.c |   53 ++++++++++++++-------------------------------
 libglusterfs/src/timer.h |    2 +-
 2 files changed, 18 insertions(+), 37 deletions(-)

diff --git a/libglusterfs/src/timer.c b/libglusterfs/src/timer.c
index 8344c9b..a4d1890 100644
--- a/libglusterfs/src/timer.c
+++ b/libglusterfs/src/timer.c
@@ -85,31 +85,11 @@ gf_timer_call_after (glusterfs_ctx_t *ctx,
 }
 
 int32_t
-gf_timer_call_stale (gf_timer_registry_t *reg,
-                     gf_timer_t *event)
-{
-        if (reg == NULL || event == NULL)
-        {
-                gf_msg_callingfn ("timer", GF_LOG_ERROR, EINVAL,
-                                  LG_MSG_INVALID_ARG, "invalid argument");
-                return 0;
-        }
-
-        event->next->prev = event->prev;
-        event->prev->next = event->next;
-        event->next = &reg->stale;
-        event->prev = event->next->prev;
-        event->next->prev = event;
-        event->prev->next = event;
-
-        return 0;
-}
-
-int32_t
 gf_timer_call_cancel (glusterfs_ctx_t *ctx,
                       gf_timer_t *event)
 {
         gf_timer_registry_t *reg = NULL;
+        gf_boolean_t fired = _gf_false;
 
         if (ctx == NULL || event == NULL)
         {
@@ -128,13 +108,21 @@ gf_timer_call_cancel (glusterfs_ctx_t *ctx,
 
         pthread_mutex_lock (&reg->lock);
         {
+		fired = event->fired;
+		if (fired)
+			goto unlock;
+
                 event->next->prev = event->prev;
                 event->prev->next = event->next;
         }
+unlock:
         pthread_mutex_unlock (&reg->lock);
 
-        GF_FREE (event);
-        return 0;
+	if (!fired) {
+		GF_FREE (event);
+		return 0;
+        }
+        return -1;
 }
 
 static inline void __delete_entry (gf_timer_t *event) {
@@ -181,7 +169,9 @@ gf_timer_proc (void *ctx)
                                 at = TS (event->at);
                                 if (event != &reg->active && now >= at) {
                                         need_cbk = 1;
-                                        gf_timer_call_stale (reg, event);
+                                        event->next->prev = event->prev;
+                                        event->prev->next = event->next;
+                                        event->fired = 1;
                                 }
                         }
                         pthread_mutex_unlock (&reg->lock);
@@ -192,15 +182,13 @@ gf_timer_proc (void *ctx)
                                         THIS = event->xl;
                                 }
                                 event->callbk (event->data);
-                                /*This callbk above would have freed the event
-                                 * by calling timer_cancel, don't ever touch it
-                                 * again*/
+                                GF_FREE (event);
                                 if (old_THIS) {
                                         THIS = old_THIS;
                                 }
-                        }
-                        else
+                        } else {
                                 break;
+                        }
                 }
                 nanosleep (&sleepts, NULL);
         }
@@ -216,11 +204,6 @@ gf_timer_proc (void *ctx)
                          * list_head*/
                         __delete_entry (event);
                 }
-
-                while (reg->stale.next != &reg->stale) {
-                        event = reg->stale.next;
-                        __delete_entry (event);
-                }
         }
         pthread_mutex_unlock (&reg->lock);
         pthread_mutex_destroy (&reg->lock);
@@ -249,8 +232,6 @@ gf_timer_registry_init (glusterfs_ctx_t *ctx)
                 pthread_mutex_init (&reg->lock, NULL);
                 reg->active.next = &reg->active;
                 reg->active.prev = &reg->active;
-                reg->stale.next = &reg->stale;
-                reg->stale.prev = &reg->stale;
 
                 ctx->timer = reg;
                 gf_thread_create (&reg->th, NULL, gf_timer_proc, ctx);
diff --git a/libglusterfs/src/timer.h b/libglusterfs/src/timer.h
index e64b350..35d99be 100644
--- a/libglusterfs/src/timer.h
+++ b/libglusterfs/src/timer.h
@@ -29,12 +29,12 @@ struct _gf_timer {
         gf_timer_cbk_t    callbk;
         void             *data;
         xlator_t         *xl;
+	gf_boolean_t      fired;
 };
 
 struct _gf_timer_registry {
         pthread_t        th;
         char             fin;
-        struct _gf_timer stale;
         struct _gf_timer active;
         pthread_mutex_t  lock;
 };
-- 
1.7.1