diff options
author | Mohammed Rafi KC <rkavunga@redhat.com> | 2019-02-26 18:04:18 +0530 |
---|---|---|
committer | Amar Tumballi <amarts@redhat.com> | 2019-03-20 13:24:44 +0000 |
commit | f2f07591b2de9ba45bbc3eb4f601d1e9a327190b (patch) | |
tree | 1eb628f99b91246759e3bc1e9de1238c9fea2cc4 /rpc/rpc-lib/src | |
parent | 6d6a3b298ee81c6c7d93941365852c1bdb42c3c1 (diff) | |
download | glusterfs-f2f07591b2de9ba45bbc3eb4f601d1e9a327190b.tar.gz glusterfs-f2f07591b2de9ba45bbc3eb4f601d1e9a327190b.tar.xz glusterfs-f2f07591b2de9ba45bbc3eb4f601d1e9a327190b.zip |
rpc/transport: Missing a ref on dict while creating transport object
while creating rpc_tranpsort object, we store a dictionary without
taking a ref on dict but it does an unref during the cleaning of the
transport object.
So the rpc layer expect the caller to take a ref on the dictionary
before passing dict to rpc layer. This leads to a lot of confusion
across the code base and leads to ref leaks.
Semantically, this is not correct. It is the rpc layer responsibility
to take a ref when storing it, and free during the cleanup.
I'm listing down the total issues or leaks across the code base because
of this confusion. These issues are currently present in the upstream
master.
1) changelog_rpc_client_init
2) quota_enforcer_init
3) rpcsvc_create_listeners : when there are two transport, like tcp,rdma.
4) quotad_aggregator_init
5) glusterd: init
6) nfs3_init_state
7) server: init
8) client:init
This patch does the cleanup according to the semantics.
Change-Id: I46373af9630373eb375ee6de0e6f2bbe2a677425
updates: bz#1659708
Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
Diffstat (limited to 'rpc/rpc-lib/src')
-rw-r--r-- | rpc/rpc-lib/src/rpc-clnt.c | 2 | ||||
-rw-r--r-- | rpc/rpc-lib/src/rpc-transport.c | 38 | ||||
-rw-r--r-- | rpc/rpc-lib/src/rpc-transport.h | 4 | ||||
-rw-r--r-- | rpc/rpc-lib/src/rpcsvc.c | 13 | ||||
-rw-r--r-- | rpc/rpc-lib/src/rpcsvc.h | 2 |
5 files changed, 16 insertions, 43 deletions
diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c index 6f47515a48..b04eaed40d 100644 --- a/rpc/rpc-lib/src/rpc-clnt.c +++ b/rpc/rpc-lib/src/rpc-clnt.c @@ -1125,8 +1125,6 @@ rpc_clnt_new(dict_t *options, xlator_t *owner, char *name, mem_pool_destroy(rpc->saved_frames_pool); GF_FREE(rpc); rpc = NULL; - if (options) - dict_unref(options); goto out; } diff --git a/rpc/rpc-lib/src/rpc-transport.c b/rpc/rpc-lib/src/rpc-transport.c index ce984426cb..baa8593f6c 100644 --- a/rpc/rpc-lib/src/rpc-transport.c +++ b/rpc/rpc-lib/src/rpc-transport.c @@ -170,6 +170,11 @@ rpc_transport_cleanup(rpc_transport_t *trans) if (trans->fini) trans->fini(trans); + if (trans->options) { + dict_unref(trans->options); + trans->options = NULL; + } + GF_FREE(trans->name); if (trans->xl) @@ -354,7 +359,7 @@ rpc_transport_load(glusterfs_ctx_t *ctx, dict_t *options, char *trans_name) } } - trans->options = options; + trans->options = dict_ref(options); pthread_mutex_init(&trans->lock, NULL); trans->xl = this; @@ -593,19 +598,14 @@ out: } int -rpc_transport_unix_options_build(dict_t **options, char *filepath, +rpc_transport_unix_options_build(dict_t *dict, char *filepath, int frame_timeout) { - dict_t *dict = NULL; char *fpath = NULL; int ret = -1; GF_ASSERT(filepath); - GF_ASSERT(options); - - dict = dict_new(); - if (!dict) - goto out; + GF_VALIDATE_OR_GOTO("rpc-transport", dict, out); fpath = gf_strdup(filepath); if (!fpath) { @@ -640,20 +640,14 @@ rpc_transport_unix_options_build(dict_t **options, char *filepath, if (ret) goto out; } - - *options = dict; out: - if (ret && dict) { - dict_unref(dict); - } return ret; } int -rpc_transport_inet_options_build(dict_t **options, const char *hostname, - int port, char *af) +rpc_transport_inet_options_build(dict_t *dict, const char *hostname, int port, + char *af) { - dict_t *dict = NULL; char *host = NULL; int ret = -1; #ifdef IPV6_DEFAULT @@ -662,13 +656,9 @@ rpc_transport_inet_options_build(dict_t **options, const char *hostname, char *addr_family = "inet"; #endif - GF_ASSERT(options); GF_ASSERT(hostname); GF_ASSERT(port >= 1024); - - dict = dict_new(); - if (!dict) - goto out; + GF_VALIDATE_OR_GOTO("rpc-transport", dict, out); host = gf_strdup((char *)hostname); if (!host) { @@ -704,12 +694,6 @@ rpc_transport_inet_options_build(dict_t **options, const char *hostname, "failed to set trans-type with socket"); goto out; } - - *options = dict; out: - if (ret && dict) { - dict_unref(dict); - } - return ret; } diff --git a/rpc/rpc-lib/src/rpc-transport.h b/rpc/rpc-lib/src/rpc-transport.h index 6830279f07..5b88be5a3d 100644 --- a/rpc/rpc-lib/src/rpc-transport.h +++ b/rpc/rpc-lib/src/rpc-transport.h @@ -306,11 +306,11 @@ rpc_transport_keepalive_options_set(dict_t *options, int32_t interval, int32_t time, int32_t timeout); int -rpc_transport_unix_options_build(dict_t **options, char *filepath, +rpc_transport_unix_options_build(dict_t *options, char *filepath, int frame_timeout); int -rpc_transport_inet_options_build(dict_t **options, const char *hostname, +rpc_transport_inet_options_build(dict_t *options, const char *hostname, int port, char *af); void diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c index c38a675b8c..d62e06cb18 100644 --- a/rpc/rpc-lib/src/rpcsvc.c +++ b/rpc/rpc-lib/src/rpcsvc.c @@ -2620,18 +2620,13 @@ rpcsvc_reconfigure_options(rpcsvc_t *svc, dict_t *options) } int -rpcsvc_transport_unix_options_build(dict_t **options, char *filepath) +rpcsvc_transport_unix_options_build(dict_t *dict, char *filepath) { - dict_t *dict = NULL; char *fpath = NULL; int ret = -1; GF_ASSERT(filepath); - GF_ASSERT(options); - - dict = dict_new(); - if (!dict) - goto out; + GF_VALIDATE_OR_GOTO("rpcsvc", dict, out); fpath = gf_strdup(filepath); if (!fpath) { @@ -2654,13 +2649,9 @@ rpcsvc_transport_unix_options_build(dict_t **options, char *filepath) ret = dict_set_str(dict, "transport-type", "socket"); if (ret) goto out; - - *options = dict; out: if (ret) { GF_FREE(fpath); - if (dict) - dict_unref(dict); } return ret; } diff --git a/rpc/rpc-lib/src/rpcsvc.h b/rpc/rpc-lib/src/rpcsvc.h index 34045ce8a4..a51edc7369 100644 --- a/rpc/rpc-lib/src/rpcsvc.h +++ b/rpc/rpc-lib/src/rpcsvc.h @@ -665,7 +665,7 @@ rpcsvc_actor_t * rpcsvc_program_actor(rpcsvc_request_t *req); int -rpcsvc_transport_unix_options_build(dict_t **options, char *filepath); +rpcsvc_transport_unix_options_build(dict_t *options, char *filepath); int rpcsvc_set_allow_insecure(rpcsvc_t *svc, dict_t *options); int |