summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMohit Agrawal <moagrawal@redhat.com>2018-11-29 19:55:39 +0530
committerAtin Mukherjee <amukherj@redhat.com>2018-12-03 11:34:35 +0000
commit46c15ea8fa98bb3d92580b192f03863c2e2a2d9c (patch)
tree25462a497b273a0f963e2090adbbd928a4bb9bbc
parentf77fb6d568616592ab25501c402c140d15235ca9 (diff)
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>
-rw-r--r--glusterfsd/src/glusterfsd.c4
-rw-r--r--libglusterfs/src/libglusterfs.sym1
-rw-r--r--libglusterfs/src/xlator.c31
-rw-r--r--libglusterfs/src/xlator.h2
-rw-r--r--rpc/rpc-lib/src/libgfrpc.sym1
-rw-r--r--rpc/rpc-lib/src/rpc-transport.c28
-rw-r--r--rpc/rpc-lib/src/rpc-transport.h3
-rw-r--r--rpc/rpc-lib/src/rpcsvc.c40
-rw-r--r--rpc/rpc-lib/src/rpcsvc.h3
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-utils.c33
-rw-r--r--xlators/protocol/server/src/server.c54
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;