From 7c3cc485054e4ede1efb358552135b432fb7047a Mon Sep 17 00:00:00 2001 From: Mohit Agrawal Date: Sat, 10 Feb 2018 12:25:15 +0530 Subject: glusterfsd: Memleak in glusterfsd process while brick mux is on Problem: At the time of stopping the volume while brick multiplex is enabled memory is not cleanup from all server side xlators. Solution: To cleanup memory for all server side xlators call fini in glusterfs_handle_terminate after send GF_EVENT_CLEANUP notification to top xlator. BUG: 1544090 Signed-off-by: Mohit Agrawal Note: Run all test-cases in separate build (https://review.gluster.org/19574) with same patch after enable brick mux forcefully, all test cases are passed. Change-Id: Ia10dc7f2605aa50f2b90b3fe4eb380ba9299e2fc --- xlators/storage/posix/src/posix-common.c | 50 ++++++++++++-------------- xlators/storage/posix/src/posix-entry-ops.c | 2 ++ xlators/storage/posix/src/posix-helpers.c | 1 + xlators/storage/posix/src/posix-inode-fd-ops.c | 15 ++++++-- xlators/storage/posix/src/posix-inode-handle.h | 6 ++++ 5 files changed, 43 insertions(+), 31 deletions(-) (limited to 'xlators/storage/posix') diff --git a/xlators/storage/posix/src/posix-common.c b/xlators/storage/posix/src/posix-common.c index 345d35e3e9a..507bfc20991 100644 --- a/xlators/storage/posix/src/posix-common.c +++ b/xlators/storage/posix/src/posix-common.c @@ -105,6 +105,7 @@ extern char *marker_xattrs[]; (lutimes (path, tv)) #endif + int32_t posix_priv (xlator_t *this) { @@ -147,9 +148,6 @@ posix_notify (xlator_t *this, void *data, ...) { - struct posix_private *priv = NULL; - - priv = this->private; switch (event) { case GF_EVENT_PARENT_UP: @@ -157,31 +155,6 @@ posix_notify (xlator_t *this, /* Tell the parent that posix xlator is up */ default_notify (this, GF_EVENT_CHILD_UP, data); } - break; - case GF_EVENT_CLEANUP: - if (priv->health_check) { - priv->health_check_active = _gf_false; - pthread_cancel (priv->health_check); - priv->health_check = 0; - } - if (priv->disk_space_check) { - priv->disk_space_check_active = _gf_false; - pthread_cancel (priv->disk_space_check); - priv->disk_space_check = 0; - } - if (priv->janitor) { - (void) gf_thread_cleanup_xint (priv->janitor); - priv->janitor = 0; - } - if (priv->fsyncer) { - (void) gf_thread_cleanup_xint (priv->fsyncer); - priv->fsyncer = 0; - } - if (priv->mount_lock) { - (void) sys_closedir (priv->mount_lock); - priv->mount_lock = NULL; - } - break; default: /* */ @@ -1133,11 +1106,32 @@ posix_fini (xlator_t *this) if (!priv) return; this->private = NULL; + if (priv->health_check) { + priv->health_check_active = _gf_false; + pthread_cancel (priv->health_check); + priv->health_check = 0; + } + if (priv->disk_space_check) { + priv->disk_space_check_active = _gf_false; + pthread_cancel (priv->disk_space_check); + priv->disk_space_check = 0; + } + if (priv->janitor) { + (void) gf_thread_cleanup_xint (priv->janitor); + priv->janitor = 0; + } + if (priv->fsyncer) { + (void) gf_thread_cleanup_xint (priv->fsyncer); + priv->fsyncer = 0; + } /*unlock brick dir*/ if (priv->mount_lock) (void) sys_closedir (priv->mount_lock); GF_FREE (priv->base_path); + LOCK_DESTROY (&priv->lock); + pthread_mutex_destroy (&priv->janitor_lock); + pthread_mutex_destroy (&priv->fsync_mutex); GF_FREE (priv->hostname); GF_FREE (priv->trash_path); GF_FREE (priv); diff --git a/xlators/storage/posix/src/posix-entry-ops.c b/xlators/storage/posix/src/posix-entry-ops.c index 7463bf9eb45..69e125a45ab 100644 --- a/xlators/storage/posix/src/posix-entry-ops.c +++ b/xlators/storage/posix/src/posix-entry-ops.c @@ -186,6 +186,7 @@ posix_lookup (call_frame_t *frame, xlator_t *this, VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this, out); VALIDATE_OR_GOTO (loc, out); + VALIDATE_OR_GOTO (this->private, out); priv = this->private; @@ -1052,6 +1053,7 @@ posix_unlink (call_frame_t *frame, xlator_t *this, VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this, out); + VALIDATE_OR_GOTO (this->private, out); VALIDATE_OR_GOTO (loc, out); SET_FS_ID (frame->root->uid, frame->root->gid); diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c index 56c9b4afe94..377389d22a3 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -2060,6 +2060,7 @@ abort: gf_log (THIS->name, GF_LOG_INFO, "detaching not-only " " child %s", priv->base_path); top->notify (top, GF_EVENT_CLEANUP, victim); + xlator_mem_cleanup (victim); } } diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c index f3a2a7bfb83..bca7419eee0 100644 --- a/xlators/storage/posix/src/posix-inode-fd-ops.c +++ b/xlators/storage/posix/src/posix-inode-fd-ops.c @@ -1104,6 +1104,8 @@ posix_releasedir (xlator_t *this, } priv = this->private; + if (!priv) + goto out; pthread_mutex_lock (&priv->janitor_lock); { @@ -1838,6 +1840,8 @@ posix_release (xlator_t *this, fd_t *fd) "pfd->dir is %p (not NULL) for file fd=%p", pfd->dir, fd); } + if (!priv) + goto out; pthread_mutex_lock (&priv->janitor_lock); { @@ -2025,6 +2029,7 @@ posix_setxattr (call_frame_t *frame, xlator_t *this, VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this, out); + VALIDATE_OR_GOTO (this->private, out); VALIDATE_OR_GOTO (loc, out); VALIDATE_OR_GOTO (dict, out); @@ -2610,6 +2615,7 @@ posix_getxattr (call_frame_t *frame, xlator_t *this, VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this, out); VALIDATE_OR_GOTO (loc, out); + VALIDATE_OR_GOTO (this->private, out); SET_FS_ID (frame->root->uid, frame->root->gid); MAKE_INODE_HANDLE (real_path, this, loc, NULL); @@ -2723,11 +2729,12 @@ posix_getxattr (call_frame_t *frame, xlator_t *this, goto done; } if (loc->inode && name && (XATTR_IS_PATHINFO (name))) { - if (LOC_HAS_ABSPATH (loc)) + VALIDATE_OR_GOTO (this->private, out); + if (LOC_HAS_ABSPATH (loc)) { MAKE_REAL_PATH (rpath, this, loc->path); - else + } else { rpath = real_path; - + } size = gf_asprintf (&host_buf, "", priv->base_path, ((priv->node_uuid_pathinfo && @@ -4986,6 +4993,8 @@ posix_forget (xlator_t *this, inode_t *inode) struct posix_private *priv_posix = NULL; priv_posix = (struct posix_private *) this->private; + if (!priv_posix) + return 0; ret = inode_ctx_del (inode, this, &ctx_uint); if (!ctx_uint) diff --git a/xlators/storage/posix/src/posix-inode-handle.h b/xlators/storage/posix/src/posix-inode-handle.h index 6849276d3db..b6cb871e007 100644 --- a/xlators/storage/posix/src/posix-inode-handle.h +++ b/xlators/storage/posix/src/posix-inode-handle.h @@ -55,6 +55,12 @@ } while (0) #define MAKE_INODE_HANDLE(rpath, this, loc, iatt_p) do { \ + if (!this->private) { \ + gf_msg ("make_inode_handle", GF_LOG_ERROR, 0, \ + P_MSG_INODE_HANDLE_CREATE, \ + "private is NULL, fini is already called"); \ + break; \ + } \ if (gf_uuid_is_null (loc->gfid)) { \ gf_msg (this->name, GF_LOG_ERROR, 0, \ P_MSG_INODE_HANDLE_CREATE, \ -- cgit