summaryrefslogtreecommitdiffstats
path: root/xlators/mgmt/glusterd
diff options
context:
space:
mode:
authorMohammed Rafi KC <rkavunga@redhat.com>2019-02-26 18:04:18 +0530
committerAmar Tumballi <amarts@redhat.com>2019-03-20 13:24:44 +0000
commitf2f07591b2de9ba45bbc3eb4f601d1e9a327190b (patch)
tree1eb628f99b91246759e3bc1e9de1238c9fea2cc4 /xlators/mgmt/glusterd
parent6d6a3b298ee81c6c7d93941365852c1bdb42c3c1 (diff)
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 'xlators/mgmt/glusterd')
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-conn-mgmt.c8
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-handler.c18
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-rebalance.c8
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-utils.c9
-rw-r--r--xlators/mgmt/glusterd/src/glusterd.c6
5 files changed, 38 insertions, 11 deletions
diff --git a/xlators/mgmt/glusterd/src/glusterd-conn-mgmt.c b/xlators/mgmt/glusterd/src/glusterd-conn-mgmt.c
index e80e152cd58..c6d7a00b06d 100644
--- a/xlators/mgmt/glusterd/src/glusterd-conn-mgmt.c
+++ b/xlators/mgmt/glusterd/src/glusterd-conn-mgmt.c
@@ -29,6 +29,10 @@ glusterd_conn_init(glusterd_conn_t *conn, char *sockpath, int frame_timeout,
if (!this)
goto out;
+ options = dict_new();
+ if (!options)
+ goto out;
+
svc = glusterd_conn_get_svc_object(conn);
if (!svc) {
gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_SVC_GET_FAIL,
@@ -36,7 +40,7 @@ glusterd_conn_init(glusterd_conn_t *conn, char *sockpath, int frame_timeout,
goto out;
}
- ret = rpc_transport_unix_options_build(&options, sockpath, frame_timeout);
+ ret = rpc_transport_unix_options_build(options, sockpath, frame_timeout);
if (ret)
goto out;
@@ -66,6 +70,8 @@ glusterd_conn_init(glusterd_conn_t *conn, char *sockpath, int frame_timeout,
conn->rpc = rpc;
conn->notify = notify;
out:
+ if (options)
+ dict_unref(options);
if (ret) {
if (rpc) {
rpc_clnt_unref(rpc);
diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c
index c3830589ce7..b63fa77478a 100644
--- a/xlators/mgmt/glusterd/src/glusterd-handler.c
+++ b/xlators/mgmt/glusterd/src/glusterd-handler.c
@@ -3429,11 +3429,10 @@ out:
}
int
-glusterd_transport_inet_options_build(dict_t **options, const char *hostname,
+glusterd_transport_inet_options_build(dict_t *dict, const char *hostname,
int port, char *af)
{
xlator_t *this = NULL;
- dict_t *dict = NULL;
int32_t interval = -1;
int32_t time = -1;
int32_t timeout = -1;
@@ -3441,14 +3440,14 @@ glusterd_transport_inet_options_build(dict_t **options, const char *hostname,
this = THIS;
GF_ASSERT(this);
- GF_ASSERT(options);
+ GF_ASSERT(dict);
GF_ASSERT(hostname);
if (!port)
port = GLUSTERD_DEFAULT_PORT;
/* Build default transport options */
- ret = rpc_transport_inet_options_build(&dict, hostname, port, af);
+ ret = rpc_transport_inet_options_build(dict, hostname, port, af);
if (ret)
goto out;
@@ -3488,7 +3487,6 @@ glusterd_transport_inet_options_build(dict_t **options, const char *hostname,
if ((interval > 0) || (time > 0))
ret = rpc_transport_keepalive_options_set(dict, interval, time,
timeout);
- *options = dict;
out:
gf_msg_debug("glusterd", 0, "Returning %d", ret);
return ret;
@@ -3508,6 +3506,10 @@ glusterd_friend_rpc_create(xlator_t *this, glusterd_peerinfo_t *peerinfo,
if (!peerctx)
goto out;
+ options = dict_new();
+ if (!options)
+ goto out;
+
if (args)
peerctx->args = *args;
@@ -3522,7 +3524,7 @@ glusterd_friend_rpc_create(xlator_t *this, glusterd_peerinfo_t *peerinfo,
if (ret)
gf_log(this->name, GF_LOG_TRACE,
"option transport.address-family is not set in xlator options");
- ret = glusterd_transport_inet_options_build(&options, peerinfo->hostname,
+ ret = glusterd_transport_inet_options_build(options, peerinfo->hostname,
peerinfo->port, af);
if (ret)
goto out;
@@ -3532,6 +3534,7 @@ glusterd_friend_rpc_create(xlator_t *this, glusterd_peerinfo_t *peerinfo,
* create our RPC endpoint with the same address that the peer would
* use to reach us.
*/
+
if (this->options) {
data = dict_getn(this->options, "transport.socket.bind-address",
SLEN("transport.socket.bind-address"));
@@ -3573,6 +3576,9 @@ glusterd_friend_rpc_create(xlator_t *this, glusterd_peerinfo_t *peerinfo,
peerctx = NULL;
ret = 0;
out:
+ if (options)
+ dict_unref(options);
+
GF_FREE(peerctx);
return ret;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
index 34b0294474c..5562ebbda37 100644
--- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c
+++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
@@ -391,6 +391,10 @@ glusterd_rebalance_rpc_create(glusterd_volinfo_t *volinfo)
if (!defrag)
goto out;
+ options = dict_new();
+ if (!options)
+ goto out;
+
GLUSTERD_GET_DEFRAG_SOCK_FILE(sockfile, volinfo);
/* Check if defrag sockfile exists in the new location
* in /var/run/ , if it does not try the old location
@@ -420,7 +424,7 @@ glusterd_rebalance_rpc_create(glusterd_volinfo_t *volinfo)
* default timeout of 30mins used for unreliable network connections is
* too long for unix domain socket connections.
*/
- ret = rpc_transport_unix_options_build(&options, sockfile, 600);
+ ret = rpc_transport_unix_options_build(options, sockfile, 600);
if (ret) {
gf_msg(THIS->name, GF_LOG_ERROR, 0, GD_MSG_UNIX_OP_BUILD_FAIL,
"Unix options build failed");
@@ -437,6 +441,8 @@ glusterd_rebalance_rpc_create(glusterd_volinfo_t *volinfo)
}
ret = 0;
out:
+ if (options)
+ dict_unref(options);
return ret;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index e8d4e1e35f0..dd4c02422e3 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -1974,7 +1974,11 @@ glusterd_brick_connect(glusterd_volinfo_t *volinfo,
* The default timeout of 30mins used for unreliable network
* connections is too long for unix domain socket connections.
*/
- ret = rpc_transport_unix_options_build(&options, socketpath, 600);
+ options = dict_new();
+ if (!options)
+ goto out;
+
+ ret = rpc_transport_unix_options_build(options, socketpath, 600);
if (ret)
goto out;
@@ -1993,7 +1997,8 @@ glusterd_brick_connect(glusterd_volinfo_t *volinfo,
brickinfo->rpc = rpc;
}
out:
-
+ if (options)
+ dict_unref(options);
gf_msg_debug("glusterd", 0, "Returning %d", ret);
return ret;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index ff5af42079d..280c9f3640e 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -1111,11 +1111,15 @@ glusterd_init_uds_listener(xlator_t *this)
GF_ASSERT(this);
+ options = dict_new();
+ if (!options)
+ goto out;
+
sock_data = dict_get(this->options, "glusterd-sockfile");
(void)snprintf(sockfile, sizeof(sockfile), "%s",
sock_data ? sock_data->data : DEFAULT_GLUSTERD_SOCKFILE);
- ret = rpcsvc_transport_unix_options_build(&options, sockfile);
+ ret = rpcsvc_transport_unix_options_build(options, sockfile);
if (ret)
goto out;