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