Bug 1136831

+++ This bug was initially created as a clone of Bug #1134221 +++

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 }

--- Additional comment from Anand Avati on 2014-08-27 06:09:29 EDT ---

REVIEW: http://review.gluster.org/8553 (cluster/afr: Fix dict_t leaks) posted (#1) for review on master by Krutika Dhananjay (kdhananj)

--- Additional comment from Anand Avati on 2014-08-28 02:12:47 EDT ---

REVIEW: http://review.gluster.org/8557 (statedump: Print curr_stdalloc in mempool statedump ...) posted (#1) for review on master by Krutika Dhananjay (kdhananj)

--- Additional comment from Anand Avati on 2014-08-28 02:31:46 EDT ---

REVIEW: http://review.gluster.org/8553 (cluster/afr: Fix dict_t leaks) posted (#2) for review on master by Krutika Dhananjay (kdhananj)

--- Additional comment from Anand Avati on 2014-08-28 03:55:36 EDT ---

REVIEW: http://review.gluster.org/8557 (statedump: Print curr_stdalloc in mempool statedump ...) posted (#2) for review on master by Krutika Dhananjay (kdhananj)

--- Additional comment from Anand Avati on 2014-08-28 04:19:52 EDT ---

COMMIT: http://review.gluster.org/8553 committed in master by Pranith Kumar Karampuri (pkarampu) 
------
commit dc844c545caa7f2cf08fd71caa5051348a5f3c78
Author: Krutika Dhananjay <kdhananj>
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>
    Reviewed-on: http://review.gluster.org/8553
    Reviewed-by: Pranith Kumar Karampuri <pkarampu>
    Tested-by: Pranith Kumar Karampuri <pkarampu>
    Tested-by: Gluster Build System <jenkins.com>

--- Additional comment from Anand Avati on 2014-08-29 22:31:54 EDT ---

COMMIT: http://review.gluster.org/8557 committed in master by Pranith Kumar Karampuri (pkarampu) 
------
commit f52efa681b1a16c287ed00e2a79cc7f05e65fed1
Author: Krutika Dhananjay <kdhananj>
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>
    Reviewed-on: http://review.gluster.org/8557
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Humble Devassy Chirammal <humble.devassy>
    Reviewed-by: Pranith Kumar Karampuri <pkarampu>
    Tested-by: Pranith Kumar Karampuri <pkarampu>