Summary:	ASTERISK-16509: Another sip_peer structure leak
Reporter:	Kirill Katsnelson (kkm)	Labels:
Date Opened:	2010-08-04 21:58:06	Date Closed:	2010-08-23 17:03:28
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	I found a few places in chan_sip.c where multiple peers are removed by a call to ao2_t_callback(peers,..OBJ_UNLINK, ...). There is no corresponding attempt to removed them also from the peers_by_ip list: In sip_prune_realtime() near line 15884 In reload_config() near line 26188 In sip_do_reload() near line 27747 Interesting that in another branch of sip_prune_realtime(), when a single peers is removed, it is unlinked from peers_by_ip. Also, in build_peer() near line 25448, a single peer is removed from peers but not peers_by_ip(). ****** ADDITIONAL INFORMATION ****** sip_peers also leak for other reasons. This is not the only extant reference. Tracking down another one. Russel Bryant on the dev list: “I'm not intimately familiar with that code, but it sounds like a bug.  Please open an issue on issues.asterisk.org so that we can investigate further.“
Comments:	By: Digium Subversion (svnbot) 2010-08-19 15:31:21 Repository: asterisk Revision: 282890 U   branches/1.6.2/channels/chan_sip.c ------------------------------------------------------------------------ r282890 | dvossel | 2010-08-19 15:31:21 -0500 (Thu, 19 Aug 2010) | 5 lines fixes sip peer memory leaks in the peer_by_ip table (issue ASTERISK-16509) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=282890 By: Digium Subversion (svnbot) 2010-08-19 15:34:39 Repository: asterisk Revision: 282891 _U  branches/1.8/ U   branches/1.8/channels/chan_sip.c ------------------------------------------------------------------------ r282891 | dvossel | 2010-08-19 15:34:39 -0500 (Thu, 19 Aug 2010) | 11 lines Merged revisions 282890 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ........  r282890 | dvossel | 2010-08-19 15:31:22 -0500 (Thu, 19 Aug 2010) | 5 lines    fixes sip peer memory leaks in the peer_by_ip table    (issue ASTERISK-16509) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=282891 By: Digium Subversion (svnbot) 2010-08-19 15:35:41 Repository: asterisk Revision: 282892 _U  trunk/ U   trunk/channels/chan_sip.c ------------------------------------------------------------------------ r282892 | dvossel | 2010-08-19 15:35:40 -0500 (Thu, 19 Aug 2010) | 18 lines Merged revisions 282891 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ................  r282891 | dvossel | 2010-08-19 15:34:41 -0500 (Thu, 19 Aug 2010) | 11 lines    Merged revisions 282890 via svnmerge from  https://origsvn.digium.com/svn/asterisk/branches/1.6.2    ........    r282890 | dvossel | 2010-08-19 15:31:22 -0500 (Thu, 19 Aug 2010) | 5 lines        fixes sip peer memory leaks in the peer_by_ip table        (issue ASTERISK-16509)  ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=282892 By: David Vossel (dvossel) 2010-08-19 15:36:09 The patch i just committed fixed everything i could find regarding the peer_by_ip table.  Am i missing anything? By: Kirill Katsnelson (kkm) 2010-08-19 17:29:08 I do not think that I am qualified to usefully comment on this patch. Seems fine to me but do not take my approval too seriously. :-) By: David Vossel (dvossel) 2010-08-19 17:32:19 Well, can you tell if my patch addressed all the areas you were concerned about? By: Kirill Katsnelson (kkm) 2010-08-19 17:42:07 There were no practical implications on asterisk performance or something that I can positively test fixed; I only spotted that while eyeballing the code when I was debugging a patch (for something not directly related). When I asked on the dev list why are not object always removed from the list by IP, Russel responded that it is probably a bug, asked me to open an issue, and so I did. I'd be glad to be of more help here, but I do not really see what else can I can do. By: David Vossel (dvossel) 2010-08-23 10:02:14 awesome, well I will close the issue then.  Thanks for the report!