protocol/server: add and remove the transports from the list, inside the lock

Till now for graph changes, glusterfs client used to remember the old graph
also. Hence the transport object on the server corresponding the old graph
never received disconnect. But now since the graph cleanup is happening,
transport on the server side gets disconnect for the cleaned up graph.

Server maintains, all the transports in a list. But addition of the new
transport to the list, or removal of the transport from the list is not
happening within the lock. Thus if a thread is accessing a transport
(in cases of statedump, where each transprt's information is dumped),
and the server gets a disconnect on that transport, then it leads to
segfault of the process.

To avoid it do the list (of transports) manipulation inside the lock.

Change-Id: I50e8389d5ec8f1c52b8d401ef8c8ddd262e82548
BUG: 803815
Signed-off-by: Raghavendra Bhat <raghavendrabhat@gluster.com>
Reviewed-on: http://review.gluster.com/2958
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

2 files changed, 54 insertions, 26 deletions

diff --git a/xlators/protocol/server/src/server-helpers.c b/xlators/protocol/server/src/server-helpers.c
index 7613912e137..40b1efc9b95 100644
--- a/xlators/protocol/server/src/server-helpers.c
+++ b/xlators/protocol/server/src/server-helpers.c
@@ -1370,10 +1370,14 @@ gf_server_check_getxattr_cmd (call_frame_t *frame, const char *key)
 
         if (fnmatch ("*list*mount*point*", key, 0) == 0) {
                 /* list all the client protocol connecting to this process */
-                list_for_each_entry (xprt, &conf->xprt_list, list) {
-                        gf_log ("mount-point-list", GF_LOG_INFO,
-                                "%s", xprt->peerinfo.identifier);
+                pthread_mutex_lock (&conf->mutex);
+                {
+                        list_for_each_entry (xprt, &conf->xprt_list, list) {
+                                gf_log ("mount-point-list", GF_LOG_INFO,
+                                        "%s", xprt->peerinfo.identifier);
+                        }
                 }
+                pthread_mutex_unlock (&conf->mutex);
         }
 
         /* Add more options/keys here */
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index b75ad172e6f..f87e371f1bb 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -298,28 +298,40 @@ server_priv_to_dict (xlator_t *this, dict_t *dict)
                 return 0;
         //TODO: Dump only specific info to dict
 
-        list_for_each_entry (xprt, &conf->xprt_list, list) {
-                peerinfo = &xprt->peerinfo;
-                memset (key, 0, sizeof (key));
-                snprintf (key, sizeof (key), "client%d.hostname", count);
-                ret = dict_set_str (dict, key, peerinfo->identifier);
-                if (ret)
-                        goto out;
+        pthread_mutex_lock (&conf->mutex);
+        {
+                list_for_each_entry (xprt, &conf->xprt_list, list) {
+                        peerinfo = &xprt->peerinfo;
+                        memset (key, 0, sizeof (key));
+                        snprintf (key, sizeof (key), "client%d.hostname",
+                                  count);
+                        ret = dict_set_str (dict, key, peerinfo->identifier);
+                        if (ret)
+                                goto unlock;
 
-                memset (key, 0, sizeof (key));
-                snprintf (key, sizeof (key), "client%d.bytesread", count);
-                ret = dict_set_uint64 (dict, key, xprt->total_bytes_read);
-                if (ret)
-                        goto out;
+                        memset (key, 0, sizeof (key));
+                        snprintf (key, sizeof (key), "client%d.bytesread",
+                                  count);
+                        ret = dict_set_uint64 (dict, key,
+                                               xprt->total_bytes_read);
+                        if (ret)
+                                goto unlock;
 
-                memset (key, 0, sizeof (key));
-                snprintf (key, sizeof (key), "client%d.byteswrite", count);
-                ret = dict_set_uint64 (dict, key, xprt->total_bytes_write);
-                if (ret)
-                        goto out;
+                        memset (key, 0, sizeof (key));
+                        snprintf (key, sizeof (key), "client%d.byteswrite",
+                                  count);
+                        ret = dict_set_uint64 (dict, key,
+                                               xprt->total_bytes_write);
+                        if (ret)
+                                goto unlock;
 
-                count++;
+                        count++;
+                }
         }
+unlock:
+        pthread_mutex_unlock (&conf->mutex);
+        if (ret)
+                goto out;
 
         ret = dict_set_int32 (dict, "clientcount", count);
 
@@ -343,10 +355,14 @@ server_priv (xlator_t *this)
         if (!conf)
                 return 0;
 
-        list_for_each_entry (xprt, &conf->xprt_list, list) {
-                total_read  += xprt->total_bytes_read;
-                total_write += xprt->total_bytes_write;
+        pthread_mutex_lock (&conf->mutex);
+        {
+                list_for_each_entry (xprt, &conf->xprt_list, list) {
+                        total_read  += xprt->total_bytes_read;
+                        total_write += xprt->total_bytes_write;
+                }
         }
+        pthread_mutex_unlock (&conf->mutex);
 
         gf_proc_dump_build_key(key, "server", "total-bytes-read");
         gf_proc_dump_write(key, "%"PRIu64, total_read);
@@ -613,7 +629,11 @@ server_rpc_notify (rpcsvc_t *rpc, void *xl, rpcsvc_event_t event,
                 */
                 INIT_LIST_HEAD (&xprt->list);
 
-                list_add_tail (&xprt->list, &conf->xprt_list);
+                pthread_mutex_lock (&conf->mutex);
+                {
+                        list_add_tail (&xprt->list, &conf->xprt_list);
+                }
+                pthread_mutex_unlock (&conf->mutex);
 
                 break;
         }
@@ -625,7 +645,11 @@ server_rpc_notify (rpcsvc_t *rpc, void *xl, rpcsvc_event_t event,
                 put_server_conn_state (this, xprt);
                 gf_log (this->name, GF_LOG_INFO, "disconnecting connection"
                         "from %s", xprt->peerinfo.identifier);
-                list_del (&xprt->list);
+                pthread_mutex_lock (&conf->mutex);
+                {
+                        list_del (&xprt->list);
+                }
+                pthread_mutex_unlock (&conf->mutex);
 
                 pthread_mutex_lock (&conn->lock);
                 {