From dc844c545caa7f2cf08fd71caa5051348a5f3c78 Mon Sep 17 00:00:00 2001 From: Krutika Dhananjay Date: Wed, 27 Aug 2014 15:14:04 +0530 Subject: cluster/afr: Fix dict_t leaks dict_t objects that are ref'd in alloca'd "replies" in afr_replies_copy() are not unref'd after "replies" go out of scope. Change-Id: Id5a6ca3c17a8de72b94b3e0f92165609da5a36ea BUG: 1134221 Signed-off-by: Krutika Dhananjay Reviewed-on: http://review.gluster.org/8553 Reviewed-by: Pranith Kumar Karampuri Tested-by: Pranith Kumar Karampuri Tested-by: Gluster Build System --- xlators/cluster/afr/src/afr-common.c | 33 ++++++++++++++---------- xlators/cluster/afr/src/afr-self-heal-common.c | 3 +++ xlators/cluster/afr/src/afr-self-heal-data.c | 4 +++ xlators/cluster/afr/src/afr-self-heal-entry.c | 15 ++++++----- xlators/cluster/afr/src/afr-self-heal-metadata.c | 2 ++ xlators/cluster/afr/src/afr-self-heal-name.c | 30 +++++++++++++-------- xlators/cluster/afr/src/afr-self-heal.h | 6 ++--- xlators/cluster/afr/src/afr.h | 6 ++++- 8 files changed, 64 insertions(+), 35 deletions(-) (limited to 'xlators/cluster/afr/src') diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index cc7df9a3ea6..8ab67af405f 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -472,7 +472,7 @@ afr_refresh_selfheal_wrap (void *opaque) err = afr_inode_refresh_err (frame, this); - afr_replies_wipe (local, this->private); + afr_local_replies_wipe (local, this->private); local->refreshfn (frame, this, err); @@ -509,7 +509,7 @@ afr_inode_refresh_done (call_frame_t *frame, xlator_t *this) err = afr_inode_refresh_err (frame, this); - afr_replies_wipe (local, this->private); + afr_local_replies_wipe (local, this->private); if (ret && afr_selfheal_enabled (this)) { heal = copy_frame (frame); @@ -588,7 +588,7 @@ afr_inode_refresh_do (call_frame_t *frame, xlator_t *this) priv = this->private; local = frame->local; - afr_replies_wipe (local, priv); + afr_local_replies_wipe (local, priv); xdata = dict_new (); if (!xdata) { @@ -877,19 +877,26 @@ afr_local_transaction_cleanup (afr_local_t *local, xlator_t *this) void -afr_replies_wipe (afr_local_t *local, afr_private_t *priv) +afr_replies_wipe (struct afr_reply *replies, int count) +{ + int i = 0; + + for (i = 0; i < count; i++) { + if (replies[i].xdata) { + dict_unref (replies[i].xdata); + replies[i].xdata = NULL; + } + } +} + +void +afr_local_replies_wipe (afr_local_t *local, afr_private_t *priv) { - int i; if (!local->replies) return; - for (i = 0; i < priv->child_count; i++) { - if (local->replies[i].xdata) { - dict_unref (local->replies[i].xdata); - local->replies[i].xdata = NULL; - } - } + afr_replies_wipe (local->replies, priv->child_count); memset (local->replies, 0, sizeof(*local->replies) * priv->child_count); } @@ -934,7 +941,7 @@ afr_local_cleanup (afr_local_t *local, xlator_t *this) if (local->dict) dict_unref (local->dict); - afr_replies_wipe (local, priv); + afr_local_replies_wipe (local, priv); GF_FREE(local->replies); GF_FREE (local->child_up); @@ -1446,7 +1453,7 @@ afr_lookup_selfheal_wrap (void *opaque) afr_selfheal_name (frame->this, local->loc.pargfid, local->loc.name, &local->cont.lookup.gfid_req); - afr_replies_wipe (local, this->private); + afr_local_replies_wipe (local, this->private); inode = afr_selfheal_unlocked_lookup_on (frame, local->loc.parent, local->loc.name, local->replies, diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c index 1d0831f0752..1c3d4a5dc86 100644 --- a/xlators/cluster/afr/src/afr-self-heal-common.c +++ b/xlators/cluster/afr/src/afr-self-heal-common.c @@ -916,6 +916,9 @@ afr_selfheal_unlocked_inspect (call_frame_t *frame, xlator_t *this, out: if (inode) inode_unref (inode); + if (replies) + afr_replies_wipe (replies, priv->child_count); + return ret; } diff --git a/xlators/cluster/afr/src/afr-self-heal-data.c b/xlators/cluster/afr/src/afr-self-heal-data.c index 402474e787b..a8a2607dbe9 100644 --- a/xlators/cluster/afr/src/afr-self-heal-data.c +++ b/xlators/cluster/afr/src/afr-self-heal-data.c @@ -557,6 +557,10 @@ out: if (compat) afr_selfheal_uninodelk (frame, this, fd->inode, this->name, LLONG_MAX - 2, 1, compat_lock); + + if (locked_replies) + afr_replies_wipe (locked_replies, priv->child_count); + return ret; } diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c index dc1546c7403..97397c1b098 100644 --- a/xlators/cluster/afr/src/afr-self-heal-entry.c +++ b/xlators/cluster/afr/src/afr-self-heal-entry.c @@ -342,6 +342,8 @@ unlock: locked_on); if (inode) inode_unref (inode); + if (replies) + afr_replies_wipe (replies, priv->child_count); return ret; } @@ -404,8 +406,7 @@ afr_selfheal_entry_do_subvol (call_frame_t *frame, xlator_t *this, fd_t *fd, static int afr_selfheal_entry_do (call_frame_t *frame, xlator_t *this, fd_t *fd, int source, unsigned char *sources, - unsigned char *healed_sinks, - struct afr_reply *locked_replies) + unsigned char *healed_sinks) { int i = 0; afr_private_t *priv = NULL; @@ -431,8 +432,7 @@ afr_selfheal_entry_do (call_frame_t *frame, xlator_t *this, fd_t *fd, static int __afr_selfheal_entry_finalize_source (xlator_t *this, unsigned char *sources, unsigned char *healed_sinks, - unsigned char *locked_on, - struct afr_reply *replies) + unsigned char *locked_on) { int i = 0; afr_private_t *priv = NULL; @@ -493,8 +493,7 @@ __afr_selfheal_entry_prepare (call_frame_t *frame, xlator_t *this, fd_t *fd, AFR_INTERSECT (healed_sinks, sinks, locked_on, priv->child_count); source = __afr_selfheal_entry_finalize_source (this, sources, - healed_sinks, - locked_on, replies); + healed_sinks, locked_on); if (source < 0) { /* If source is < 0 (typically split-brain), we perform a conservative merge of entries rather than erroring out */ @@ -546,7 +545,7 @@ unlock: goto out; ret = afr_selfheal_entry_do (frame, this, fd, source, sources, - healed_sinks, locked_replies); + healed_sinks); if (ret) goto out; @@ -554,6 +553,8 @@ unlock: healed_sinks, AFR_ENTRY_TRANSACTION, locked_replies, data_lock); out: + if (locked_replies) + afr_replies_wipe (locked_replies, priv->child_count); return ret; } diff --git a/xlators/cluster/afr/src/afr-self-heal-metadata.c b/xlators/cluster/afr/src/afr-self-heal-metadata.c index 683fb2dd60a..efbfd63aa21 100644 --- a/xlators/cluster/afr/src/afr-self-heal-metadata.c +++ b/xlators/cluster/afr/src/afr-self-heal-metadata.c @@ -236,6 +236,8 @@ __afr_selfheal_metadata (call_frame_t *frame, xlator_t *this, inode_t *inode, unlock: afr_selfheal_uninodelk (frame, this, inode, this->name, LLONG_MAX -1, 0, data_lock); + if (locked_replies) + afr_replies_wipe (locked_replies, priv->child_count); return ret; } diff --git a/xlators/cluster/afr/src/afr-self-heal-name.c b/xlators/cluster/afr/src/afr-self-heal-name.c index f09eb0be6ba..f1626cc034e 100644 --- a/xlators/cluster/afr/src/afr-self-heal-name.c +++ b/xlators/cluster/afr/src/afr-self-heal-name.c @@ -264,8 +264,7 @@ __afr_selfheal_name_do (call_frame_t *frame, xlator_t *this, inode_t *parent, int __afr_selfheal_name_finalize_source (xlator_t *this, unsigned char *sources, unsigned char *healed_sinks, - unsigned char *locked_on, - struct afr_reply *replies) + unsigned char *locked_on) { int i = 0; afr_private_t *priv = NULL; @@ -296,24 +295,26 @@ int __afr_selfheal_name_prepare (call_frame_t *frame, xlator_t *this, inode_t *parent, uuid_t pargfid, unsigned char *locked_on, unsigned char *sources, unsigned char *sinks, - unsigned char *healed_sinks, struct afr_reply *replies, - int *source_p) + unsigned char *healed_sinks, int *source_p) { int ret = -1; int source = -1; afr_private_t *priv = NULL; + struct afr_reply *replies = NULL; priv = this->private; + replies = alloca0 (priv->child_count * sizeof(*replies)); + ret = afr_selfheal_unlocked_discover (frame, parent, pargfid, replies); if (ret) - return ret; + goto out; ret = afr_selfheal_find_direction (frame, this, replies, AFR_ENTRY_TRANSACTION, locked_on, sources, sinks); if (ret) - return ret; + goto out; /* Initialize the healed_sinks[] array optimistically to the intersection of to-be-healed (i.e sinks[]) and @@ -326,14 +327,17 @@ __afr_selfheal_name_prepare (call_frame_t *frame, xlator_t *this, inode_t *paren AFR_INTERSECT (healed_sinks, sinks, locked_on, priv->child_count); source = __afr_selfheal_name_finalize_source (this, sources, - healed_sinks, - locked_on, replies); + healed_sinks, locked_on); if (source < 0) { /* If source is < 0 (typically split-brain), we perform a conservative merge of entries rather than erroring out */ } *source_p = source; +out: + if (replies) + afr_replies_wipe (replies, priv->child_count); + return ret; } @@ -348,7 +352,7 @@ afr_selfheal_name_do (call_frame_t *frame, xlator_t *this, inode_t *parent, unsigned char *healed_sinks = NULL; unsigned char *locked_on = NULL; int source = -1; - struct afr_reply *replies = NULL; + struct afr_reply *replies = NULL; int ret = -1; inode_t *inode = NULL; @@ -371,8 +375,7 @@ afr_selfheal_name_do (call_frame_t *frame, xlator_t *this, inode_t *parent, ret = __afr_selfheal_name_prepare (frame, this, parent, pargfid, locked_on, sources, sinks, - healed_sinks, replies, - &source); + healed_sinks, &source); if (ret) goto unlock; @@ -394,6 +397,9 @@ unlock: if (inode) inode_unref (inode); + if (replies) + afr_replies_wipe (replies, priv->child_count); + return ret; } @@ -441,6 +447,8 @@ afr_selfheal_name_unlocked_inspect (call_frame_t *frame, xlator_t *this, if (inode) inode_unref (inode); + if (replies) + afr_replies_wipe (replies, priv->child_count); return 0; } diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h index 8556b2bb335..da0025bb600 100644 --- a/xlators/cluster/afr/src/afr-self-heal.h +++ b/xlators/cluster/afr/src/afr-self-heal.h @@ -20,7 +20,7 @@ afr_private_t *__priv = frame->this->private; \ int __i = 0, __count = 0; \ \ - afr_replies_wipe (__local, __priv); \ + afr_local_replies_wipe (__local, __priv); \ \ for (__i = 0; __i < __priv->child_count; __i++) { \ if (!__priv->child_up[__i]) continue; \ @@ -41,7 +41,7 @@ afr_private_t *__priv = frame->this->private; \ int __i = 0, __count = 0; \ \ - afr_replies_wipe (__local, __priv); \ + afr_local_replies_wipe (__local, __priv); \ \ for (__i = 0; __i < __priv->child_count; __i++) { \ if (!list[__i]) continue; \ @@ -59,7 +59,7 @@ afr_private_t *__priv = frame->this->private; \ int __i = 0; \ \ - afr_replies_wipe (__local, __priv); \ + afr_local_replies_wipe (__local, __priv); \ \ for (__i = 0; __i < __priv->child_count; __i++) { \ if (!__priv->child_up[__i]) continue; \ diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index 35d4d545bc9..8e7bea79712 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -821,7 +821,7 @@ int afr_replies_interpret (call_frame_t *frame, xlator_t *this, inode_t *inode); void -afr_replies_wipe (afr_local_t *local, afr_private_t *priv); +afr_local_replies_wipe (afr_local_t *local, afr_private_t *priv); void afr_local_cleanup (afr_local_t *local, xlator_t *this); @@ -964,4 +964,8 @@ afr_local_pathinfo (char *pathinfo, gf_boolean_t *is_local); void afr_remove_eager_lock_stub (afr_local_t *local); + +void +afr_replies_wipe (struct afr_reply *replies, int count); + #endif /* __AFR_H__ */ -- cgit