server: Resolve memory leak path in server_init

Problem: 1) server_init does not cleanup allocate resources
            while it is failed before return error
         2) dict leak at the time of graph destroying

Solution: 1) free resources in case of server_init is failed
          2) Take dict_ref of graph xlator before destroying
             the graph to avoid leak

Change-Id: I9e31e156b9ed6bebe622745a8be0e470774e3d15
fixes: bz#1654917
Signed-off-by: Mohit Agrawal <moagrawal@redhat.com>

11 files changed, 151 insertions, 49 deletions

diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c
index 771af423ea5..5bf0db0edfb 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -2618,6 +2618,10 @@ out:
             xl = graph->first;
             if ((ctx->active != graph) &&
                 (xl && !strcmp(xl->type, "protocol/server"))) {
+                /* Take dict ref for every graph xlator to avoid dict leak
+                   at the time of graph destroying
+                */
+                gluster_graph_take_reference(graph->first);
                 glusterfs_graph_fini(graph);
                 glusterfs_graph_destroy(graph);
             }
diff --git a/libglusterfs/src/libglusterfs.sym b/libglusterfs/src/libglusterfs.sym
index c8c42311c80..baf44de64ad 100644
--- a/libglusterfs/src/libglusterfs.sym
+++ b/libglusterfs/src/libglusterfs.sym
@@ -1121,6 +1121,7 @@ xlator_volume_option_get
 xlator_volume_option_get_list
 xlator_memrec_free
 xlator_mem_cleanup
+gluster_graph_take_reference
 default_fops
 gf_fop_list
 gf_upcall_list
diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c
index cbbe8cf7f12..dedce05d5c3 100644
--- a/libglusterfs/src/xlator.c
+++ b/libglusterfs/src/xlator.c
@@ -1555,3 +1555,34 @@ glusterfs_delete_volfile_checksum(glusterfs_ctx_t *ctx, const char *volfile_id)
 
     return 0;
 }
+
+/*
+   The function is required to take dict ref for every xlator at graph.
+   At the time of compare graph topology create a graph and populate
+   key values in the dictionary, after finished graph comparison we do destroy
+   the new graph.At the time of construct graph we don't take any reference
+   so to avoid dict leak at the of destroying graph due to ref counter underflow
+   we need to call dict_ref here.
+
+*/
+
+void
+gluster_graph_take_reference(xlator_t *tree)
+{
+    xlator_t *trav = tree;
+    xlator_t *prev = tree;
+
+    if (!tree) {
+        gf_msg("parser", GF_LOG_ERROR, 0, LG_MSG_TREE_NOT_FOUND,
+               "Translator tree not found");
+        return;
+    }
+
+    while (prev) {
+        trav = prev->next;
+        if (prev->options)
+            dict_ref(prev->options);
+        prev = trav;
+    }
+    return;
+}
diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h
index 64a61ac0bed..30c9e5875e5 100644
--- a/libglusterfs/src/xlator.h
+++ b/libglusterfs/src/xlator.h
@@ -1082,4 +1082,6 @@ xlator_mem_cleanup(xlator_t *this);
 void
 handle_default_options(xlator_t *xl, dict_t *options);
 
+void
+gluster_graph_take_reference(xlator_t *tree);
 #endif /* _XLATOR_H */
diff --git a/rpc/rpc-lib/src/libgfrpc.sym b/rpc/rpc-lib/src/libgfrpc.sym
index 4f42485044f..f3544e3741a 100644
--- a/rpc/rpc-lib/src/libgfrpc.sym
+++ b/rpc/rpc-lib/src/libgfrpc.sym
@@ -26,6 +26,7 @@ rpcsvc_drc_priv
 rpcsvc_drc_reconfigure
 rpcsvc_get_program_vector_sizer
 rpcsvc_init
+rpcsvc_destroy
 rpcsvc_init_options
 rpcsvc_listener_destroy
 rpcsvc_program_register
diff --git a/rpc/rpc-lib/src/rpc-transport.c b/rpc/rpc-lib/src/rpc-transport.c
index 54636dcbf00..8bb6b595175 100644
--- a/rpc/rpc-lib/src/rpc-transport.c
+++ b/rpc/rpc-lib/src/rpc-transport.c
@@ -159,6 +159,24 @@ out:
     return msg;
 }
 
+void
+rpc_transport_cleanup(rpc_transport_t *trans)
+{
+    if (!trans)
+        return;
+
+    trans->fini(trans);
+    GF_FREE(trans->name);
+
+    if (trans->xl)
+        pthread_mutex_destroy(&trans->lock);
+
+    if (trans->dl_handle)
+        dlclose(trans->dl_handle);
+
+    GF_FREE(trans);
+}
+
 rpc_transport_t *
 rpc_transport_load(glusterfs_ctx_t *ctx, dict_t *options, char *trans_name)
 {
@@ -354,15 +372,7 @@ rpc_transport_load(glusterfs_ctx_t *ctx, dict_t *options, char *trans_name)
 
 fail:
     if (!success) {
-        if (trans) {
-            GF_FREE(trans->name);
-
-            if (trans->dl_handle)
-                dlclose(trans->dl_handle);
-
-            GF_FREE(trans);
-        }
-
+        rpc_transport_cleanup(trans);
         GF_FREE(name);
 
         return_trans = NULL;
diff --git a/rpc/rpc-lib/src/rpc-transport.h b/rpc/rpc-lib/src/rpc-transport.h
index 87ea3a28dfd..d7b86b63748 100644
--- a/rpc/rpc-lib/src/rpc-transport.h
+++ b/rpc/rpc-lib/src/rpc-transport.h
@@ -308,4 +308,7 @@ rpc_transport_unix_options_build(dict_t **options, char *filepath,
 int
 rpc_transport_inet_options_build(dict_t **options, const char *hostname,
                                  int port);
+
+void
+rpc_transport_cleanup(rpc_transport_t *);
 #endif /* __RPC_TRANSPORT_H__ */
diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c
index 8dcc2947b33..fd531fbc1ee 100644
--- a/rpc/rpc-lib/src/rpcsvc.c
+++ b/rpc/rpc-lib/src/rpcsvc.c
@@ -37,6 +37,7 @@
 #include <stdarg.h>
 #include <stdio.h>
 #include <math.h>
+#include <dlfcn.h>
 
 #ifdef IPV6_DEFAULT
 #include <netconfig.h>
@@ -2009,6 +2010,7 @@ rpcsvc_create_listener(rpcsvc_t *svc, dict_t *options, char *name)
 
     listener = rpcsvc_listener_alloc(svc, trans);
     if (listener == NULL) {
+        ret = -1;
         goto out;
     }
 
@@ -2016,6 +2018,7 @@ rpcsvc_create_listener(rpcsvc_t *svc, dict_t *options, char *name)
 out:
     if (!listener && trans) {
         rpc_transport_disconnect(trans, _gf_true);
+        rpc_transport_cleanup(trans);
     }
 
     return ret;
@@ -2747,6 +2750,43 @@ rpcsvc_get_throttle(rpcsvc_t *svc)
     return svc->throttle;
 }
 
+/* Function call to cleanup resources for svc
+ */
+int
+rpcsvc_destroy(rpcsvc_t *svc)
+{
+    struct rpcsvc_auth_list *auth = NULL;
+    struct rpcsvc_auth_list *tmp = NULL;
+    rpcsvc_listener_t *listener = NULL;
+    rpcsvc_listener_t *next = NULL;
+    int ret = 0;
+
+    if (!svc)
+        return ret;
+
+    list_for_each_entry_safe(listener, next, &svc->listeners, list)
+    {
+        rpcsvc_listener_destroy(listener);
+    }
+
+    list_for_each_entry_safe(auth, tmp, &svc->authschemes, authlist)
+    {
+        list_del_init(&auth->authlist);
+        GF_FREE(auth);
+    }
+
+    rpcsvc_program_unregister(svc, &gluster_dump_prog);
+    if (svc->rxpool) {
+        mem_pool_destroy(svc->rxpool);
+        svc->rxpool = NULL;
+    }
+
+    pthread_rwlock_destroy(&svc->rpclock);
+    GF_FREE(svc);
+
+    return ret;
+}
+
 /* The global RPC service initializer.
  */
 rpcsvc_t *
diff --git a/rpc/rpc-lib/src/rpcsvc.h b/rpc/rpc-lib/src/rpcsvc.h
index d6260ca5028..b296f9a4bde 100644
--- a/rpc/rpc-lib/src/rpcsvc.h
+++ b/rpc/rpc-lib/src/rpcsvc.h
@@ -677,4 +677,7 @@ rpcsvc_get_program_vector_sizer(rpcsvc_t *svc, uint32_t prognum,
                                 uint32_t progver, int procnum);
 void
 rpcsvc_autoscale_threads(glusterfs_ctx_t *ctx, rpcsvc_t *rpc, int incr);
+
+extern int
+rpcsvc_destroy(rpcsvc_t *svc);
 #endif
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 2deda2945c0..cdac6d5b8bf 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -9301,35 +9301,6 @@ glusterd_defrag_volume_status_update(glusterd_volinfo_t *volinfo,
 
     return ret;
 }
-/*
-   The function is required to take dict ref for every xlator at graph.
-   At the time of compare graph topology create a graph and populate
-   key values in the dictionary, after finished graph comparison we do destroy
-   the new graph.At the time of construct graph we don't take any reference
-   so to avoid leak due to ref counter underflow we need to call dict_ref here.
-
-*/
-
-void
-glusterd_graph_take_reference(xlator_t *tree)
-{
-    xlator_t *trav = tree;
-    xlator_t *prev = tree;
-
-    if (!tree) {
-        gf_msg("parser", GF_LOG_ERROR, 0, LG_MSG_TREE_NOT_FOUND,
-               "Translator tree not found");
-        return;
-    }
-
-    while (prev) {
-        trav = prev->next;
-        if (prev->options)
-            dict_ref(prev->options);
-        prev = trav;
-    }
-    return;
-}
 
 int
 glusterd_check_topology_identical(const char *filename1, const char *filename2,
@@ -9376,14 +9347,14 @@ glusterd_check_topology_identical(const char *filename1, const char *filename2,
     if (grph1 == NULL)
         goto out;
 
-    glusterd_graph_take_reference(grph1->first);
+    gluster_graph_take_reference(grph1->first);
 
     /* create the graph for filename2 */
     grph2 = glusterfs_graph_construct(fp2);
     if (grph2 == NULL)
         goto out;
 
-    glusterd_graph_take_reference(grph2->first);
+    gluster_graph_take_reference(grph2->first);
 
     /* compare the graph topology */
     *identical = is_graph_topology_equal(grph1, grph2);
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index 77e5d74e7c5..81aa72fbbc0 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -986,6 +986,49 @@ server_dump_metrics(xlator_t *this, int fd)
     return 0;
 }
 
+void
+server_cleanup(xlator_t *this, server_conf_t *conf)
+{
+    if (!this || !conf)
+        return;
+
+    LOCK_DESTROY(&conf->itable_lock);
+    pthread_mutex_destroy(&conf->mutex);
+
+    if (this->ctx->event_pool) {
+        /* Free the event pool */
+        (void)event_pool_destroy(this->ctx->event_pool);
+    }
+
+    if (dict_get(this->options, "config-directory")) {
+        GF_FREE(conf->conf_dir);
+        conf->conf_dir = NULL;
+    }
+
+    if (conf->child_status) {
+        GF_FREE(conf->child_status);
+        conf->child_status = NULL;
+    }
+
+    if (this->ctx->statedump_path) {
+        GF_FREE(this->ctx->statedump_path);
+        this->ctx->statedump_path = NULL;
+    }
+
+    if (conf->auth_modules) {
+        gf_auth_fini(conf->auth_modules);
+        dict_unref(conf->auth_modules);
+    }
+
+    if (conf->rpc) {
+        (void)rpcsvc_destroy(conf->rpc);
+        conf->rpc = NULL;
+    }
+
+    GF_FREE(conf);
+    this->private = NULL;
+}
+
 int
 server_init(xlator_t *this)
 {
@@ -1061,6 +1104,7 @@ server_init(xlator_t *this)
     ret = gf_auth_init(this, conf->auth_modules);
     if (ret) {
         dict_unref(conf->auth_modules);
+        conf->auth_modules = NULL;
         goto out;
     }
 
@@ -1239,15 +1283,7 @@ out:
         if (this != NULL) {
             this->fini(this);
         }
-
-        if (conf && conf->rpc) {
-            rpcsvc_listener_t *listener, *next;
-            list_for_each_entry_safe(listener, next, &conf->rpc->listeners,
-                                     list)
-            {
-                rpcsvc_listener_destroy(listener);
-            }
-        }
+        server_cleanup(this, conf);
     }
 
     return ret;