graph/cleanup: Fix race in graph cleanup

We were unconditionally cleaning up the grap when we get
child_down followed by parent_down. But this is prone to
race condition when some of the bricks are already disconnected.
In this case, even before the last child down is executed in the
client xlator code,we might have freed the graph. Because the
child_down event is alreadt recevied.

To fix this race, we have introduced a check to see if all client
xlator have cleared thier reconnect chain, and called the child_down
for last time.

Change-Id: I7d02813bc366dac733a836e0cd7b14a6fac52042
fixes: bz#1727329
Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>

6 files changed, 33 insertions, 2 deletions

diff --git a/libglusterfs/src/defaults-tmpl.c b/libglusterfs/src/defaults-tmpl.c
index 82e7f78d7f3..3cf707f42aa 100644
--- a/libglusterfs/src/defaults-tmpl.c
+++ b/libglusterfs/src/defaults-tmpl.c
@@ -171,8 +171,11 @@ default_notify(xlator_t *this, int32_t event, void *data, ...)
                 /* Make sure this is not a daemon with master xlator */
                 pthread_mutex_lock(&graph->mutex);
                 {
-                    graph->used = 0;
-                    pthread_cond_broadcast(&graph->child_down_cond);
+                    if (graph->parent_down ==
+                        graph_total_client_xlator(graph)) {
+                        graph->used = 0;
+                        pthread_cond_broadcast(&graph->child_down_cond);
+                    }
                 }
                 pthread_mutex_unlock(&graph->mutex);
             }
diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h
index 01262dcd9f5..155bf435386 100644
--- a/libglusterfs/src/glusterfs/glusterfs.h
+++ b/libglusterfs/src/glusterfs/glusterfs.h
@@ -594,6 +594,7 @@ struct _glusterfs_graph {
                       in client multiplexed code path */
     pthread_mutex_t mutex;
     pthread_cond_t child_down_cond; /* for broadcasting CHILD_DOWN */
+    int parent_down;
     char graph_uuid[128];
 };
 typedef struct _glusterfs_graph glusterfs_graph_t;
diff --git a/libglusterfs/src/glusterfs/xlator.h b/libglusterfs/src/glusterfs/xlator.h
index 6449e59f484..6608d6cdf0d 100644
--- a/libglusterfs/src/glusterfs/xlator.h
+++ b/libglusterfs/src/glusterfs/xlator.h
@@ -1095,4 +1095,6 @@ mgmt_is_multiplexed_daemon(char *name);
 
 gf_boolean_t
 xlator_is_cleanup_starting(xlator_t *this);
+int
+graph_total_client_xlator(glusterfs_graph_t *graph);
 #endif /* _XLATOR_H */
diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c
index bbc5ad68d94..e6ae40db2ed 100644
--- a/libglusterfs/src/graph.c
+++ b/libglusterfs/src/graph.c
@@ -1695,6 +1695,7 @@ glusterfs_process_svc_attach_volfp(glusterfs_ctx_t *ctx, FILE *fp,
                "failed to construct the graph");
         goto out;
     }
+    graph->parent_down = 0;
     graph->last_xl = glusterfs_get_last_xlator(graph);
 
     for (xl = graph->first; xl; xl = xl->next) {
diff --git a/libglusterfs/src/libglusterfs.sym b/libglusterfs/src/libglusterfs.sym
index 2e83d3f1003..dc7382ba749 100644
--- a/libglusterfs/src/libglusterfs.sym
+++ b/libglusterfs/src/libglusterfs.sym
@@ -1169,3 +1169,4 @@ glusterfs_process_svc_detach
 mgmt_is_multiplexed_daemon
 xlator_is_cleanup_starting
 gf_nanosleep
+graph_total_client_xlator
diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c
index 9906809f7aa..8605fbd0e6f 100644
--- a/libglusterfs/src/xlator.c
+++ b/libglusterfs/src/xlator.c
@@ -1542,3 +1542,26 @@ xlator_is_cleanup_starting(xlator_t *this)
 out:
     return cleanup;
 }
+
+int
+graph_total_client_xlator(glusterfs_graph_t *graph)
+{
+    xlator_t *xl = NULL;
+    int count = 0;
+
+    if (!graph) {
+        gf_msg("xlator", GF_LOG_WARNING, EINVAL, LG_MSG_INVALID_ARG,
+               "graph object is null");
+        goto out;
+    }
+
+    xl = graph->first;
+    while (xl) {
+        if (strcmp(xl->type, "protocol/client") == 0) {
+            count++;
+        }
+        xl = xl->next;
+    }
+out:
+    return count;
+}