|
|
74b1de |
From 4d95e271a9042bf2d789a4d900ad263b6ea47681 Mon Sep 17 00:00:00 2001
|
|
|
74b1de |
From: Mohammed Rafi KC <rkavunga@redhat.com>
|
|
|
74b1de |
Date: Wed, 23 Jan 2019 21:55:01 +0530
|
|
|
74b1de |
Subject: [PATCH 100/124] clnt/rpc: ref leak during disconnect.
|
|
|
74b1de |
|
|
|
74b1de |
During disconnect cleanup, we are not cancelling reconnect
|
|
|
74b1de |
timer, which causes a ref leak each time when a disconnect
|
|
|
74b1de |
happen.
|
|
|
74b1de |
|
|
|
74b1de |
Backport of: https://review.gluster.org/#/c/glusterfs/+/22087/
|
|
|
74b1de |
|
|
|
74b1de |
>Change-Id: I9d05d1f368d080e04836bf6a0bb018bf8f7b5b8a
|
|
|
74b1de |
>updates: bz#1659708
|
|
|
74b1de |
>Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
|
|
|
74b1de |
|
|
|
74b1de |
Change-Id: I5a2dbb17e663a4809bb4c435cacadbf0ab694a76
|
|
|
74b1de |
BUG: 1471742
|
|
|
74b1de |
Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
|
|
|
74b1de |
Reviewed-on: https://code.engineering.redhat.com/gerrit/167844
|
|
|
74b1de |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
74b1de |
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
|
|
|
74b1de |
---
|
|
|
74b1de |
libglusterfs/src/timer.c | 16 +++++++----
|
|
|
74b1de |
rpc/rpc-lib/src/rpc-clnt.c | 11 +++++++-
|
|
|
74b1de |
.../mgmt/glusterd/src/glusterd-snapshot-utils.c | 32 ++++++++++++++++++----
|
|
|
74b1de |
3 files changed, 47 insertions(+), 12 deletions(-)
|
|
|
74b1de |
|
|
|
74b1de |
diff --git a/libglusterfs/src/timer.c b/libglusterfs/src/timer.c
|
|
|
74b1de |
index d882543..2643c07 100644
|
|
|
74b1de |
--- a/libglusterfs/src/timer.c
|
|
|
74b1de |
+++ b/libglusterfs/src/timer.c
|
|
|
74b1de |
@@ -75,13 +75,13 @@ gf_timer_call_cancel(glusterfs_ctx_t *ctx, gf_timer_t *event)
|
|
|
74b1de |
if (ctx == NULL || event == NULL) {
|
|
|
74b1de |
gf_msg_callingfn("timer", GF_LOG_ERROR, EINVAL, LG_MSG_INVALID_ARG,
|
|
|
74b1de |
"invalid argument");
|
|
|
74b1de |
- return 0;
|
|
|
74b1de |
+ return -1;
|
|
|
74b1de |
}
|
|
|
74b1de |
|
|
|
74b1de |
if (ctx->cleanup_started) {
|
|
|
74b1de |
gf_msg_callingfn("timer", GF_LOG_INFO, 0, LG_MSG_CTX_CLEANUP_STARTED,
|
|
|
74b1de |
"ctx cleanup started");
|
|
|
74b1de |
- return 0;
|
|
|
74b1de |
+ return -1;
|
|
|
74b1de |
}
|
|
|
74b1de |
|
|
|
74b1de |
LOCK(&ctx->lock);
|
|
|
74b1de |
@@ -93,10 +93,9 @@ gf_timer_call_cancel(glusterfs_ctx_t *ctx, gf_timer_t *event)
|
|
|
74b1de |
if (!reg) {
|
|
|
74b1de |
/* This can happen when cleanup may have just started and
|
|
|
74b1de |
* gf_timer_registry_destroy() sets ctx->timer to NULL.
|
|
|
74b1de |
- * Just bail out as success as gf_timer_proc() takes
|
|
|
74b1de |
- * care of cleaning up the events.
|
|
|
74b1de |
+ * gf_timer_proc() takes care of cleaning up the events.
|
|
|
74b1de |
*/
|
|
|
74b1de |
- return 0;
|
|
|
74b1de |
+ return -1;
|
|
|
74b1de |
}
|
|
|
74b1de |
|
|
|
74b1de |
LOCK(®->lock);
|
|
|
74b1de |
@@ -203,6 +202,13 @@ gf_timer_proc(void *data)
|
|
|
74b1de |
list_for_each_entry_safe(event, tmp, ®->active, list)
|
|
|
74b1de |
{
|
|
|
74b1de |
list_del(&event->list);
|
|
|
74b1de |
+ /* TODO Possible resource leak
|
|
|
74b1de |
+ * Before freeing the event, we need to call the respective
|
|
|
74b1de |
+ * event functions and free any resources.
|
|
|
74b1de |
+ * For example, In case of rpc_clnt_reconnect, we need to
|
|
|
74b1de |
+ * unref rpc object which was taken when added to timer
|
|
|
74b1de |
+ * wheel.
|
|
|
74b1de |
+ */
|
|
|
74b1de |
GF_FREE(event);
|
|
|
74b1de |
}
|
|
|
74b1de |
}
|
|
|
74b1de |
diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c
|
|
|
74b1de |
index 3f7bb3c..6f47515 100644
|
|
|
74b1de |
--- a/rpc/rpc-lib/src/rpc-clnt.c
|
|
|
74b1de |
+++ b/rpc/rpc-lib/src/rpc-clnt.c
|
|
|
74b1de |
@@ -495,6 +495,7 @@ rpc_clnt_connection_cleanup(rpc_clnt_connection_t *conn)
|
|
|
74b1de |
int unref = 0;
|
|
|
74b1de |
int ret = 0;
|
|
|
74b1de |
gf_boolean_t timer_unref = _gf_false;
|
|
|
74b1de |
+ gf_boolean_t reconnect_unref = _gf_false;
|
|
|
74b1de |
|
|
|
74b1de |
if (!conn) {
|
|
|
74b1de |
goto out;
|
|
|
74b1de |
@@ -514,6 +515,12 @@ rpc_clnt_connection_cleanup(rpc_clnt_connection_t *conn)
|
|
|
74b1de |
timer_unref = _gf_true;
|
|
|
74b1de |
conn->timer = NULL;
|
|
|
74b1de |
}
|
|
|
74b1de |
+ if (conn->reconnect) {
|
|
|
74b1de |
+ ret = gf_timer_call_cancel(clnt->ctx, conn->reconnect);
|
|
|
74b1de |
+ if (!ret)
|
|
|
74b1de |
+ reconnect_unref = _gf_true;
|
|
|
74b1de |
+ conn->reconnect = NULL;
|
|
|
74b1de |
+ }
|
|
|
74b1de |
|
|
|
74b1de |
conn->connected = 0;
|
|
|
74b1de |
conn->disconnected = 1;
|
|
|
74b1de |
@@ -533,6 +540,8 @@ rpc_clnt_connection_cleanup(rpc_clnt_connection_t *conn)
|
|
|
74b1de |
if (timer_unref)
|
|
|
74b1de |
rpc_clnt_unref(clnt);
|
|
|
74b1de |
|
|
|
74b1de |
+ if (reconnect_unref)
|
|
|
74b1de |
+ rpc_clnt_unref(clnt);
|
|
|
74b1de |
out:
|
|
|
74b1de |
return 0;
|
|
|
74b1de |
}
|
|
|
74b1de |
@@ -830,7 +839,7 @@ rpc_clnt_handle_disconnect(struct rpc_clnt *clnt, rpc_clnt_connection_t *conn)
|
|
|
74b1de |
pthread_mutex_lock(&conn->lock);
|
|
|
74b1de |
{
|
|
|
74b1de |
if (!conn->rpc_clnt->disabled && (conn->reconnect == NULL)) {
|
|
|
74b1de |
- ts.tv_sec = 10;
|
|
|
74b1de |
+ ts.tv_sec = 3;
|
|
|
74b1de |
ts.tv_nsec = 0;
|
|
|
74b1de |
|
|
|
74b1de |
rpc_clnt_ref(clnt);
|
|
|
74b1de |
diff --git a/xlators/mgmt/glusterd/src/glusterd-snapshot-utils.c b/xlators/mgmt/glusterd/src/glusterd-snapshot-utils.c
|
|
|
74b1de |
index 041946d..b3c4158 100644
|
|
|
74b1de |
--- a/xlators/mgmt/glusterd/src/glusterd-snapshot-utils.c
|
|
|
74b1de |
+++ b/xlators/mgmt/glusterd/src/glusterd-snapshot-utils.c
|
|
|
74b1de |
@@ -3364,6 +3364,25 @@ out:
|
|
|
74b1de |
return ret;
|
|
|
74b1de |
}
|
|
|
74b1de |
|
|
|
74b1de |
+int
|
|
|
74b1de |
+glusterd_is_path_mounted(const char *path)
|
|
|
74b1de |
+{
|
|
|
74b1de |
+ FILE *mtab = NULL;
|
|
|
74b1de |
+ struct mntent *part = NULL;
|
|
|
74b1de |
+ int is_mounted = 0;
|
|
|
74b1de |
+
|
|
|
74b1de |
+ if ((mtab = setmntent("/etc/mtab", "r")) != NULL) {
|
|
|
74b1de |
+ while ((part = getmntent(mtab)) != NULL) {
|
|
|
74b1de |
+ if ((part->mnt_fsname != NULL) &&
|
|
|
74b1de |
+ (strcmp(part->mnt_dir, path)) == 0) {
|
|
|
74b1de |
+ is_mounted = 1;
|
|
|
74b1de |
+ break;
|
|
|
74b1de |
+ }
|
|
|
74b1de |
+ }
|
|
|
74b1de |
+ endmntent(mtab);
|
|
|
74b1de |
+ }
|
|
|
74b1de |
+ return is_mounted;
|
|
|
74b1de |
+}
|
|
|
74b1de |
/* This function will do unmount for snaps.
|
|
|
74b1de |
*/
|
|
|
74b1de |
int32_t
|
|
|
74b1de |
@@ -3388,14 +3407,11 @@ glusterd_snap_unmount(xlator_t *this, glusterd_volinfo_t *volinfo)
|
|
|
74b1de |
continue;
|
|
|
74b1de |
}
|
|
|
74b1de |
|
|
|
74b1de |
- /* Fetch the brick mount path from the brickinfo->path */
|
|
|
74b1de |
- ret = glusterd_get_brick_root(brickinfo->path, &brick_mount_path);
|
|
|
74b1de |
+ ret = glusterd_find_brick_mount_path(brickinfo->path,
|
|
|
74b1de |
+ &brick_mount_path);
|
|
|
74b1de |
if (ret) {
|
|
|
74b1de |
- gf_msg(this->name, GF_LOG_INFO, 0, GD_MSG_BRICK_PATH_UNMOUNTED,
|
|
|
74b1de |
+ gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_BRK_MNTPATH_GET_FAIL,
|
|
|
74b1de |
"Failed to find brick_mount_path for %s", brickinfo->path);
|
|
|
74b1de |
- /* There is chance that brick path is already
|
|
|
74b1de |
- * unmounted. */
|
|
|
74b1de |
- ret = 0;
|
|
|
74b1de |
goto out;
|
|
|
74b1de |
}
|
|
|
74b1de |
/* unmount cannot be done when the brick process is still in
|
|
|
74b1de |
@@ -3440,6 +3456,10 @@ glusterd_umount(const char *path)
|
|
|
74b1de |
GF_ASSERT(this);
|
|
|
74b1de |
GF_ASSERT(path);
|
|
|
74b1de |
|
|
|
74b1de |
+ if (!glusterd_is_path_mounted(path)) {
|
|
|
74b1de |
+ return 0;
|
|
|
74b1de |
+ }
|
|
|
74b1de |
+
|
|
|
74b1de |
runinit(&runner);
|
|
|
74b1de |
snprintf(msg, sizeof(msg), "umount path %s", path);
|
|
|
74b1de |
runner_add_args(&runner, _PATH_UMOUNT, "-f", path, NULL);
|
|
|
74b1de |
--
|
|
|
74b1de |
1.8.3.1
|
|
|
74b1de |
|