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