[Home]

Summary:ASTERISK-16992: [patch] [regression] meetme conf_run leaks refs
Reporter:Mark Murawski (kobaz)Labels:
Date Opened:2010-11-20 22:23:49.000-0600Date Closed:2011-01-29 12:10:37.000-0600
Priority:MinorRegression?Yes
Status:Closed/CompleteComponents:Applications/app_meetme
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) meetme-refs.diff
Description:Affects 1.8
Affects trunk

There's two conditions (original lines 2626, 3675) where in the unlikly event of an ao2 alloc failing, we bail from conf_run and never do any cleanup.  This leaks a ref to an ast_conf_user.

Also there's a problem with a misuse of the user* pointer while in the *8 submenu (original line 3209), we lose the original *user pointer which will prevent proper cleanup on leave.

And lastly, this part at the bottom looks like it will never run.  It looks like if we get to this point, user->user_no will always have been properly initialized... and why would we want to decrement the ref count only if user_no is invalid?  We should always clean up at the end of this function.

3699 if (!user->user_no) {
3700 ao2_ref(user, -1);

So far, I think that everything in the else block of the above if could be always done on cleanup.
Comments:By: Mark Murawski (kobaz) 2010-11-20 22:30:10.000-0600

I guess technically this is a regression.  Previously ast_conf_user was not an ao2 object.  The conversion to ao2 introduced the ref leaks, when previously the cleanup was handled okay.

Changes in question.
https://reviewboard.asterisk.org/r/746/
By: Mark Murawski (kobaz) 2010-12-06 23:21:55.000-0600

Thanks for the ticket updates Leif.  I'll be able to get back on this at the end of the week.
By: Digium Subversion (svnbot) 2011-01-29 12:08:16.000-0600

Repository: asterisk
Revision: 304776

U   branches/1.6.2/apps/app_meetme.c

------------------------------------------------------------------------
r304776 | seanbright | 2011-01-29 12:08:14 -0600 (Sat, 29 Jan 2011) | 15 lines

If we fail to allocate our announcement objects, make sure we don't leak objects.

The majority of this patch was committed already in r304726 and r304729.

(issue ASTERISK-16886)
Reported by: kenji

(issue ASTERISK-17083)
Reported by: junky

(closes issue ASTERISK-16992)
Reported by: kobaz
Patches:
     meetme-refs.diff uploaded by kobaz (license 834)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=304776
By: Digium Subversion (svnbot) 2011-01-29 12:09:39.000-0600

Repository: asterisk
Revision: 304777

_U  branches/1.8/
U   branches/1.8/apps/app_meetme.c

------------------------------------------------------------------------
r304777 | seanbright | 2011-01-29 12:09:37 -0600 (Sat, 29 Jan 2011) | 22 lines

Merged revisions 304776 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

........
 r304776 | seanbright | 2011-01-29 13:08:14 -0500 (Sat, 29 Jan 2011) | 15 lines
 
 If we fail to allocate our announcement objects, make sure we don't leak objects.
 
 The majority of this patch was committed already in r304726 and r304729.
 
 (issue ASTERISK-16886)
 Reported by: kenji
 
 (issue ASTERISK-17083)
 Reported by: junky
 
 (closes issue ASTERISK-16992)
 Reported by: kobaz
 Patches:
       meetme-refs.diff uploaded by kobaz (license 834)
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=304777
By: Digium Subversion (svnbot) 2011-01-29 12:10:37.000-0600

Repository: asterisk
Revision: 304778

_U  trunk/
U   trunk/apps/app_meetme.c

------------------------------------------------------------------------
r304778 | seanbright | 2011-01-29 12:10:35 -0600 (Sat, 29 Jan 2011) | 29 lines

Merged revisions 304777 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r304777 | seanbright | 2011-01-29 13:09:37 -0500 (Sat, 29 Jan 2011) | 22 lines
 
 Merged revisions 304776 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ........
   r304776 | seanbright | 2011-01-29 13:08:14 -0500 (Sat, 29 Jan 2011) | 15 lines
   
   If we fail to allocate our announcement objects, make sure we don't leak objects.
   
   The majority of this patch was committed already in r304726 and r304729.
   
   (issue ASTERISK-16886)
   Reported by: kenji
   
   (issue ASTERISK-17083)
   Reported by: junky
   
   (closes issue ASTERISK-16992)
   Reported by: kobaz
   Patches:
         meetme-refs.diff uploaded by kobaz (license 834)
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=304778