fuse: Fix cores in notify function when this is executed in parallel

The fuse notify function gets called by the epoll or the poll thread
and till the point there is a single epoll thread, 2 notify
instances would not race with each other.

With the upcoming multi thread epoll changes, it is possible that
2 epoll threads invoke the notify function. As a result races
in this function are fixed with this commit.

The races seen are detailed in the bug, and the fix here is to
enforce a (slightly) longer critical section when updating the
fuse private structure and reserving state updates post error
handling.

Change-Id: I6974bc043cb59eb6dc39c5777123364dcefca358
BUG: 1180231
Signed-off-by: Shyam <srangana@redhat.com>
Reviewed-on: http://review.gluster.org/9421
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Tested-by: Raghavendra G <rgowdapp@redhat.com>

2 files changed, 42 insertions, 21 deletions

diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c
index cb345ad11f2..4e5adb33e44 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -5028,29 +5028,21 @@ fuse_graph_setup (xlator_t *this, glusterfs_graph_t *graph)
 
         priv = this->private;
 
-        /* handle the case of more than one CHILD_UP on same graph */
-        if (priv->active_subvol == graph->top)
-                return 0; /* This is a valid case */
-
         pthread_mutex_lock (&priv->sync_mutex);
         {
-                if (graph->used) {
-                        pthread_mutex_unlock (&priv->sync_mutex);
-                        return 0;
+                /* handle the case of more than one CHILD_UP on same graph */
+                if ((priv->active_subvol == graph->top) || graph->used) {
+                        goto unlock;
                 }
 
-                graph->used = 1;
-        }
-        pthread_mutex_unlock (&priv->sync_mutex);
-
-        itable = inode_table_new (0, graph->top);
-        if (!itable)
-                return -1;
+                itable = inode_table_new (0, graph->top);
+                if (!itable) {
+                        ret = -1;
+                        goto unlock;
+                }
 
-        ((xlator_t *)graph->top)->itable = itable;
+                ((xlator_t *)graph->top)->itable = itable;
 
-        pthread_mutex_lock (&priv->sync_mutex);
-        {
                 prev_graph = priv->next_graph;
 
                 if ((prev_graph != NULL) && (prev_graph->id > graph->id)) {
@@ -5061,12 +5053,14 @@ fuse_graph_setup (xlator_t *this, glusterfs_graph_t *graph)
                 } else {
                         priv->next_graph = graph;
                         priv->event_recvd = 0;
-
-                        pthread_cond_signal (&priv->sync_cond);
                 }
 
                 if (prev_graph != NULL)
                         winds = ((xlator_t *)prev_graph->top)->winds;
+
+		/* set post initializing next_graph i to preserve
+                 * critical section update and bails on error */
+                graph->used = 1;
         }
         pthread_mutex_unlock (&priv->sync_mutex);
 
@@ -5079,6 +5073,10 @@ fuse_graph_setup (xlator_t *this, glusterfs_graph_t *graph)
                 ((graph) ? graph->id : 0));
 
         return ret;
+unlock:
+        pthread_mutex_unlock (&priv->sync_mutex);
+
+        return ret;
 }
 
 
@@ -5087,6 +5085,7 @@ notify (xlator_t *this, int32_t event, void *data, ...)
 {
         int32_t             ret     = 0;
         fuse_private_t     *private = NULL;
+        gf_boolean_t        start_thread = _gf_false;
         glusterfs_graph_t  *graph = NULL;
 
         private = this->private;
@@ -5122,9 +5121,16 @@ notify (xlator_t *this, int32_t event, void *data, ...)
                         pthread_mutex_unlock (&private->sync_mutex);
                 }
 
-                if (!private->fuse_thread_started) {
-                        private->fuse_thread_started = 1;
+                pthread_mutex_lock (&private->sync_mutex);
+                {
+                        if (!private->fuse_thread_started) {
+                                private->fuse_thread_started = 1;
+                                start_thread = _gf_true;
+                        }
+                }
+                pthread_mutex_unlock (&private->sync_mutex);
 
+                if (start_thread) {
                         ret = gf_thread_create (&private->fuse_thread, NULL,
 						fuse_thread_proc, this);
                         if (ret != 0) {
@@ -5590,6 +5596,17 @@ fini (xlator_t *this_xl)
         if ((priv = this_xl->private) == NULL)
                 return;
 
+        pthread_mutex_lock (&priv->sync_mutex);
+        {
+                if (!(priv->fini_invoked)) {
+                        priv->fini_invoked = _gf_true;
+                } else {
+                        pthread_mutex_unlock (&priv->sync_mutex);
+                        return;
+                }
+        }
+        pthread_mutex_unlock (&priv->sync_mutex);
+
         if (dict_get (this_xl->options, ZR_MOUNTPOINT_OPT))
                 mount_point = data_to_str (dict_get (this_xl->options,
                                                      ZR_MOUNTPOINT_OPT));
diff --git a/xlators/mount/fuse/src/fuse-bridge.h b/xlators/mount/fuse/src/fuse-bridge.h
index d0d4120026b..1a0d74cd4cd 100644
--- a/xlators/mount/fuse/src/fuse-bridge.h
+++ b/xlators/mount/fuse/src/fuse-bridge.h
@@ -128,6 +128,10 @@ struct fuse_private {
 
         /* for using fuse-kernel readdirp*/
         gf_boolean_t use_readdirp;
+
+        /* fini started, helps prevent multiple epoll worker threads
+         * firing up the fini routine */
+        gf_boolean_t fini_invoked;
 };
 typedef struct fuse_private fuse_private_t;