Bug 1134221 - [AFR-V2] - dict_t leaks
Summary: [AFR-V2] - dict_t leaks
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: replicate
Version: mainline
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Krutika Dhananjay
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1136831
TreeView+ depends on / blocked
 
Reported: 2014-08-27 07:11 UTC by Krutika Dhananjay
Modified: 2015-05-14 17:43 UTC (History)
2 users (show)

Fixed In Version: glusterfs-3.7.0
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1136831 (view as bug list)
Environment:
Last Closed: 2015-05-14 17:27:23 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:


Attachments (Terms of Use)

Description Krutika Dhananjay 2014-08-27 07:11:18 UTC
Description of problem:

While reading code, I figured dict_t objects that are ref'd by 'replies' objects that are allocated via alloca(), are not unref'd when 'replies' goes out of scope. (see Additional info section for more info)


Version-Release number of selected component (if applicable):


How reproducible:
always

Steps to Reproduce:
1.
2.
3.

Actual results:
This leads to dict_t leaks.

Expected results:
There should be no leaks.

Additional info:

  8 afr_selfheal_name_unlocked_inspect (call_frame_t *frame, xlator_t *this,         
  7                                     inode_t *parent, uuid_t pargfid,             
  6                                     const char *bname, gf_boolean_t *need_heal)  
  5 {                                                                                
  4         afr_private_t *priv = NULL;                                              
  3         int i = 0;                                                               
  2         struct afr_reply *replies = NULL;                                        
  1         inode_t *inode = NULL;                                                   
  0         int first_idx = -1;                                                                                                                                                                                                                                                 
  1                                                                                  
  2         priv = this->private;                                                    
  3                                                                                  
  4         replies = alloca0 (sizeof (*replies) * priv->child_count);             <---- replies allocated via alloca()  
  5                                                                                  
  6         inode = afr_selfheal_unlocked_lookup_on (frame, parent, bname,          ****function of interest
  7                                                  replies, priv->child_up);       
  8         if (!inode)                                                              
  9                 return -ENOMEM;                                                  
 10                                                                                  
 11         for (i = 0; i < priv->child_count; i++) {                                
 12                 if (!replies[i].valid)                                           
 13                         continue;                                                
 14                                                                                  
 15                 if ((replies[i].op_ret == -1) &&                                 
 16                     (replies[i].op_errno == ENODATA))                            
 17                         *need_heal = _gf_true;                                   
 18                                                                                  
 19                 if (first_idx == -1) {                                           
 20                         first_idx = i;                                           
 21                         continue;                                                
 22                 }                                                                
 23                                                                                  
 24                 if (replies[i].op_ret != replies[first_idx].op_ret)              
 25                         *need_heal = _gf_true;                                   
 26                                                                                  
 27                 if (uuid_compare (replies[i].poststat.ia_gfid,                   
 28                                   replies[first_idx].poststat.ia_gfid))          
 29                         *need_heal = _gf_true;                                   
 30         }                                                                        
 31                                                                                  
 32         if (inode)                                                               
 33                 inode_unref (inode);                                             
 34         return 0;                                                                
 35 }              

****
 28 inode_t *                                                                        
 27 afr_selfheal_unlocked_lookup_on (call_frame_t *frame, inode_t *parent,           
 26                                  const char *name, struct afr_reply *replies,    
 25                                  unsigned char *lookup_on)                       
 24 {                                                                                
 23         loc_t loc = {0, };                                                       
 22         dict_t *xattr_req = NULL;                                                
 21         afr_local_t *local = NULL;                                               
 20         afr_private_t *priv = NULL;                                              
 19         inode_t *inode = NULL;                                                   
 18                                                                                  
 17         local = frame->local;                                                    
 16         priv = frame->this->private;                                             
 15                                                                                  
 14         xattr_req = dict_new ();                                                 
 13         if (!xattr_req)                                                          
 12                 return NULL;                                                     
 11                                                                                  
 10         if (afr_xattr_req_prepare (frame->this, xattr_req) != 0) {               
  9                 dict_destroy (xattr_req);                                        
  8                 return NULL;                                                     
  7         }                                                                        
  6                                                                                  
  5         inode = inode_new (parent->table);                                       
  4         if (!inode) {                                                            
  3                 dict_destroy (xattr_req);                                        
  2                 return NULL;                                                     
  1         }                                                                        
  0                                                                                                                                                                                                                                                                             
  1         loc.parent = inode_ref (parent);                                         
  2         uuid_copy (loc.pargfid, parent->gfid);                                   
  3         loc.name = name;                                                         
  4         loc.inode = inode_ref (inode);                                           
  5                                                                                  
  6         AFR_ONLIST (lookup_on, frame, afr_selfheal_discover_cbk, lookup, &loc,   
  7                     xattr_req);                                                  
  8                                                                                  
  9         afr_replies_copy (replies, local->replies, priv->child_count);           <--- response dicts are ref'd here into replies[i].xdata but never unrefd even after replies goes out of scope in afr_selfheal_name_unlocked_inspect()
 10                                                                                  
 11         loc_wipe (&loc);                                                         
 12         dict_unref (xattr_req);                                                  
 13                                                                                  
 14         return inode;                                                            
 15 }

Comment 1 Anand Avati 2014-08-27 10:09:29 UTC
REVIEW: http://review.gluster.org/8553 (cluster/afr: Fix dict_t leaks) posted (#1) for review on master by Krutika Dhananjay (kdhananj@redhat.com)

Comment 2 Anand Avati 2014-08-28 06:12:47 UTC
REVIEW: http://review.gluster.org/8557 (statedump: Print curr_stdalloc in mempool statedump ...) posted (#1) for review on master by Krutika Dhananjay (kdhananj@redhat.com)

Comment 3 Anand Avati 2014-08-28 06:31:46 UTC
REVIEW: http://review.gluster.org/8553 (cluster/afr: Fix dict_t leaks) posted (#2) for review on master by Krutika Dhananjay (kdhananj@redhat.com)

Comment 4 Anand Avati 2014-08-28 07:55:36 UTC
REVIEW: http://review.gluster.org/8557 (statedump: Print curr_stdalloc in mempool statedump ...) posted (#2) for review on master by Krutika Dhananjay (kdhananj@redhat.com)

Comment 5 Anand Avati 2014-08-28 08:19:52 UTC
COMMIT: http://review.gluster.org/8553 committed in master by Pranith Kumar Karampuri (pkarampu@redhat.com) 
------
commit dc844c545caa7f2cf08fd71caa5051348a5f3c78
Author: Krutika Dhananjay <kdhananj@redhat.com>
Date:   Wed Aug 27 15:14:04 2014 +0530

    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 <kdhananj@redhat.com>
    Reviewed-on: http://review.gluster.org/8553
    Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
    Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
    Tested-by: Gluster Build System <jenkins@build.gluster.com>

Comment 6 Anand Avati 2014-08-30 02:31:54 UTC
COMMIT: http://review.gluster.org/8557 committed in master by Pranith Kumar Karampuri (pkarampu@redhat.com) 
------
commit f52efa681b1a16c287ed00e2a79cc7f05e65fed1
Author: Krutika Dhananjay <kdhananj@redhat.com>
Date:   Thu Aug 28 09:58:43 2014 +0530

    statedump: Print curr_stdalloc in mempool statedump ...
    
     ...for, it is curr_stdalloc that is incremented for every mem_get()
    and decremented on every call to mem_put() and can be used to detect
    leaks, when cold_count is 0.
    
    Change-Id: I418a132c3ea4c0b99ea5c6840ff3024d8d19ddf4
    BUG: 1134221
    Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
    Reviewed-on: http://review.gluster.org/8557
    Tested-by: Gluster Build System <jenkins@build.gluster.com>
    Reviewed-by: Humble Devassy Chirammal <humble.devassy@gmail.com>
    Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
    Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

Comment 7 Anand Avati 2014-09-25 11:26:12 UTC
REVIEW: http://review.gluster.org/8852 (cluster/afr: More dict_t leak fixes) posted (#1) for review on master by Krutika Dhananjay (kdhananj@redhat.com)

Comment 8 Anand Avati 2014-09-25 12:00:02 UTC
REVIEW: http://review.gluster.org/8852 (cluster/afr: More dict_t leak fixes) posted (#2) for review on master by Krutika Dhananjay (kdhananj@redhat.com)

Comment 9 Anand Avati 2014-09-26 05:50:03 UTC
REVIEW: http://review.gluster.org/8852 (cluster/afr: More dict_t leak fixes) posted (#3) for review on master by Krutika Dhananjay (kdhananj@redhat.com)

Comment 10 Anand Avati 2014-09-26 10:06:38 UTC
COMMIT: http://review.gluster.org/8852 committed in master by Pranith Kumar Karampuri (pkarampu@redhat.com) 
------
commit 5123949bebc3520ed3d64986420c139801013c60
Author: Krutika Dhananjay <kdhananj@redhat.com>
Date:   Thu Sep 25 16:27:52 2014 +0530

    cluster/afr: More dict_t leak fixes
    
    Change-Id: I6618b7b2df0f88a3e6252f9136135cf402147a37
    BUG: 1134221
    Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
    Reviewed-on: http://review.gluster.org/8852
    Tested-by: Gluster Build System <jenkins@build.gluster.com>
    Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
    Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

Comment 11 Niels de Vos 2015-05-14 17:27:23 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.7.0, please open a new bug report.

glusterfs-3.7.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/10939
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user

Comment 12 Niels de Vos 2015-05-14 17:35:34 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.7.0, please open a new bug report.

glusterfs-3.7.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/10939
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user

Comment 13 Niels de Vos 2015-05-14 17:37:56 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.7.0, please open a new bug report.

glusterfs-3.7.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/10939
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user

Comment 14 Niels de Vos 2015-05-14 17:43:29 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.7.0, please open a new bug report.

glusterfs-3.7.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/10939
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user


Note You need to log in before you can comment on or make changes to this bug.