74b1de
From d79cb2cdff6fe8d962c9ac095a7541ddf500302b Mon Sep 17 00:00:00 2001
74b1de
From: Mohammed Rafi KC <rkavunga@redhat.com>
74b1de
Date: Mon, 1 Apr 2019 14:44:20 +0530
74b1de
Subject: [PATCH 099/124] client/fini: return fini after rpc cleanup
74b1de
74b1de
There is a race condition in rpc_transport later
74b1de
and client fini.
74b1de
74b1de
Sequence of events to happen the race condition
74b1de
1) When we want to destroy a graph, we send a parent down
74b1de
   event first
74b1de
2) Once parent down received on a client xlator, we will
74b1de
   initiates a rpc disconnect
74b1de
3) This will in turn generates a child down event.
74b1de
4) When we process child down, we first do fini for
74b1de
   Every xlator
74b1de
5) On successful return of fini, we delete the graph
74b1de
74b1de
Here after the step 5, there is a chance that the fini
74b1de
on client might not be finished. Because an rpc_tranpsort
74b1de
ref can race with the above sequence.
74b1de
74b1de
So we have to wait till all rpc's are successfully freed
74b1de
before returning the fini from client
74b1de
74b1de
Backport of: https://review.gluster.org/#/c/glusterfs/+/22468/
74b1de
74b1de
>Change-Id: I20145662d71fb837e448a4d3210d1fcb2855f2d4
74b1de
>fixes: bz#1659708
74b1de
>Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
74b1de
74b1de
Change-Id: I848bcfb9443467caed32bae0717244ab01b407fc
74b1de
BUG: 1471742
74b1de
Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
74b1de
Reviewed-on: https://code.engineering.redhat.com/gerrit/167831
74b1de
Tested-by: RHGS Build Bot <nigelb@redhat.com>
74b1de
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
74b1de
---
74b1de
 xlators/protocol/client/src/client.c | 25 ++++++++++++++++++++-----
74b1de
 xlators/protocol/client/src/client.h |  6 ++++++
74b1de
 2 files changed, 26 insertions(+), 5 deletions(-)
74b1de
74b1de
diff --git a/xlators/protocol/client/src/client.c b/xlators/protocol/client/src/client.c
74b1de
index 19f5175..a372807 100644
74b1de
--- a/xlators/protocol/client/src/client.c
74b1de
+++ b/xlators/protocol/client/src/client.c
74b1de
@@ -49,11 +49,12 @@ client_fini_complete(xlator_t *this)
74b1de
     if (!conf->destroy)
74b1de
         return 0;
74b1de
 
74b1de
-    this->private = NULL;
74b1de
-
74b1de
-    pthread_spin_destroy(&conf->fd_lock);
74b1de
-    pthread_mutex_destroy(&conf->lock);
74b1de
-    GF_FREE(conf);
74b1de
+    pthread_mutex_lock(&conf->lock);
74b1de
+    {
74b1de
+        conf->fini_completed = _gf_true;
74b1de
+        pthread_cond_broadcast(&conf->fini_complete_cond);
74b1de
+    }
74b1de
+    pthread_mutex_unlock(&conf->lock);
74b1de
 
74b1de
 out:
74b1de
     return 0;
74b1de
@@ -2721,6 +2722,7 @@ init(xlator_t *this)
74b1de
         goto out;
74b1de
 
74b1de
     pthread_mutex_init(&conf->lock, NULL);
74b1de
+    pthread_cond_init(&conf->fini_complete_cond, NULL);
74b1de
     pthread_spin_init(&conf->fd_lock, 0);
74b1de
     INIT_LIST_HEAD(&conf->saved_fds);
74b1de
 
74b1de
@@ -2779,6 +2781,7 @@ fini(xlator_t *this)
74b1de
     if (!conf)
74b1de
         return;
74b1de
 
74b1de
+    conf->fini_completed = _gf_false;
74b1de
     conf->destroy = 1;
74b1de
     if (conf->rpc) {
74b1de
         /* cleanup the saved-frames before last unref */
74b1de
@@ -2786,6 +2789,18 @@ fini(xlator_t *this)
74b1de
         rpc_clnt_unref(conf->rpc);
74b1de
     }
74b1de
 
74b1de
+    pthread_mutex_lock(&conf->lock);
74b1de
+    {
74b1de
+        while (!conf->fini_completed)
74b1de
+            pthread_cond_wait(&conf->fini_complete_cond, &conf->lock);
74b1de
+    }
74b1de
+    pthread_mutex_unlock(&conf->lock);
74b1de
+
74b1de
+    pthread_spin_destroy(&conf->fd_lock);
74b1de
+    pthread_mutex_destroy(&conf->lock);
74b1de
+    pthread_cond_destroy(&conf->fini_complete_cond);
74b1de
+    GF_FREE(conf);
74b1de
+
74b1de
     /* Saved Fds */
74b1de
     /* TODO: */
74b1de
 
74b1de
diff --git a/xlators/protocol/client/src/client.h b/xlators/protocol/client/src/client.h
74b1de
index f12fa61..8dcd72f 100644
74b1de
--- a/xlators/protocol/client/src/client.h
74b1de
+++ b/xlators/protocol/client/src/client.h
74b1de
@@ -235,6 +235,12 @@ typedef struct clnt_conf {
74b1de
                                       * up, disconnects can be
74b1de
                                       * logged
74b1de
                                       */
74b1de
+
74b1de
+    gf_boolean_t old_protocol;         /* used only for old-protocol testing */
74b1de
+    pthread_cond_t fini_complete_cond; /* Used to wait till we finsh the fini
74b1de
+                                          compltely, ie client_fini_complete
74b1de
+                                          to return*/
74b1de
+    gf_boolean_t fini_completed;
74b1de
 } clnt_conf_t;
74b1de
 
74b1de
 typedef struct _client_fd_ctx {
74b1de
-- 
74b1de
1.8.3.1
74b1de