<div dir="ltr"><div><span style="font-family:arial,sans-serif;font-size:13px">Hello Henning,</span><br></div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
Sorry hit reply only, first time.</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Would you like me still to update to use mcd_free or leave it with you?<br>
</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Best,</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
Charles</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On 4 October 2013 11:40, Henning Westerholt <span dir="ltr"><<a href="mailto:henning.westerholt@1und1.de" target="_blank">henning.westerholt@1und1.de</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Am Freitag, 4. Oktober 2013, 09:40:17 schrieb Charles Chance:<br>
<div class="im">> Yes, agreed - I'll update it today. Henning Westerholt was original author<br>
> if he'd like to review before I commit?<br>
<br>
</div>Hello Charles,<br>
<br>
I'll take a look today to it as well, thank you.<br>
<br>
Regards,<br>
<br>
Henning<br>
<div><div class="h5"><br>
> On 4 Oct 2013 09:12, "Daniel-Constantin Mierla" <<a href="mailto:miconda@gmail.com">miconda@gmail.com</a>> wrote:<br>
> > Hello,<br>
> ><br>
> > for consistency, it might be good that mcd_free() is used instead of<br>
> > pkg_free when mcd_memory is set, because it is supposed to be the pair of<br>
> > mcd_malloc() which is used to allocate the memory. mcd_free() is a wrapper<br>
> > around pkg_free(), so works like current patch, but it is safer for future<br>
> > changes to mcd_*() functions.<br>
> ><br>
> > These are remarks just looking at the current code, I am not familiar with<br>
> > memcached lib and api.<br>
> ><br>
> > Cheers,<br>
> > Daniel<br>
> ><br>
> > On 10/4/13 9:34 AM, Federico Cabiddu wrote:<br>
> ><br>
> > Hi Charles,<br>
> > I just tried the patch running the test I used to reproduce the leak.<br>
> > It's ok, the memory debug shows that the memory is correctly released.<br>
> > I tested with memory parameter set to 0 and to 1.<br>
> > Thank you.<br>
> ><br>
> > Best regards,<br>
> ><br>
> > Federico<br>
> ><br>
> > On Fri, Oct 4, 2013 at 1:00 AM, Charles Chance <<br>
> ><br>
> > <a href="mailto:charles.chance@sipcentric.com">charles.chance@sipcentric.com</a>> wrote:<br>
> >> Hi Federico/Dragos,<br>
> >><br>
> >> Thank you both for your input. I have placed the call to (pkg_)free<br>
> >><br>
> >> into a separate function and called it where necessary from each of the<br>
> >> other functions.<br>
> >><br>
> >> I haven't had chance to test yet - if you have time, please apply the<br>
> >><br>
> >> attached diff and let me know if the leak is fixed and I will commit to<br>
> >> master. Otherwise, I will test myself over the weekend.<br>
> >><br>
> >> Thanks again,<br>
> >><br>
> >> Charles<br>
> >><br>
> >> On 3 October 2013 21:30, Dragos Oancea <<a href="mailto:droancea@yahoo.com">droancea@yahoo.com</a>> wrote:<br>
> >>> Hi Charles,<br>
> >>><br>
> >>> In the function where Daniel just made the fix for the memory leak (int<br>
> >>> pv_get_mcd_value()>>><br>
> >>> ) , just before existing it with 0 , we added something like the<br>
> >>><br>
> >>> following :<br>
> >>> if (mcd_memory) {<br>
> >>><br>
> >>> pkg_free(return_value);<br>
> >>><br>
> >>> } else {<br>
> >>><br>
> >>> free(return_value);<br>
> >>><br>
> >>> }<br>
> >>><br>
> >>> It looks like it does not leak anymore, but please-double check if we<br>
> >>><br>
> >>> are free-ing it in the right place.<br>
> >>><br>
> >>> Regards,<br>
> >>><br>
> >>> Dragos<br>
> >>><br>
</div></div>> >>> ------------------------------<br>
> >>><br>
> >>> *From:* Charles Chance <<a href="mailto:charles.chance@sipcentric.com">charles.chance@sipcentric.com</a>><br>
> >>><br>
> >>> *To:* Daniel-Constantin Mierla <<a href="mailto:miconda@gmail.com">miconda@gmail.com</a>><br>
> >>> *Cc:* sr-dev <<a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a>><br>
> >>> *Sent:* Thursday, October 3, 2013 7:27 PM<br>
> >>> *Subject:* Re: [sr-dev] memory leak in memcached module<br>
<div><div class="h5">> >>><br>
> >>> I can take a look this evening. Assuming nobody has already started?<br>
> >>><br>
> >>> Best,<br>
> >>> Charles<br>
> >>><br>
> >>> On 2 Oct 2013 20:23, "Daniel-Constantin Mierla" <<a href="mailto:miconda@gmail.com">miconda@gmail.com</a>><br>
> >>><br>
> >>> wrote:<br>
> >>><br>
> >>> Hello,<br>
> >>><br>
> >>> there is (still) a memory leak in memcached module, discovered on a<br>
> >>> report by Dragos Oancea.<br>
> >>><br>
> >>> The pkg usage logs are like:<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: 19010. N address=0x7fb23683bc98<br>
> >>> frag=0x7fb23683bc68 size=8 used=1<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached:<br>
> >>> ../../parser/../ut.h: pkg_str_dup(733)<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check=<br>
> >>><br>
> >>> c0c0c0c0, abcdefed<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: 19011. N address=0x7fb23683bd00<br>
> >>><br>
> >>> frag=0x7fb23683bcd0 size=48 used=1<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached:<br>
> >>> memcached.c: mcd_malloc(127)<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check=<br>
> >>><br>
> >>> c0c0c0c0, abcdefed<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: 19012. N address=0x7fb23683bd90<br>
> >>><br>
> >>> frag=0x7fb23683bd60 size=8 used=1<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached:<br>
> >>> ../../parser/../ut.h: pkg_str_dup(733)<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check=<br>
> >>><br>
> >>> c0c0c0c0, abcdefed<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: 19013. N address=0x7fb23683bdf8<br>
> >>><br>
> >>> frag=0x7fb23683bdc8 size=48 used=1<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached:<br>
> >>> memcached.c: mcd_malloc(127)<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check=<br>
> >>><br>
> >>> c0c0c0c0, abcdefed<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: 19014. N address=0x7fb23683be88<br>
> >>><br>
> >>> frag=0x7fb23683be58 size=8 used=1<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached:<br>
> >>> memcached.c: mcd_malloc(127)<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check=<br>
> >>><br>
> >>> c0c0c0c0, abcdefed<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: 19015. N address=0x7fb23683bef0<br>
> >>><br>
> >>> frag=0x7fb23683bec0 size=16 used=1<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached:<br>
> >>> ../../parser/../ut.h: pkg_str_dup(733)<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check=<br>
> >>><br>
> >>> c0c0c0c0, abcdefed<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: 19016. N address=0x7fb23683bf60<br>
> >>><br>
> >>> frag=0x7fb23683bf30 size=8 used=1<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached:<br>
> >>> memcached.c: mcd_malloc(127)<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check=<br>
> >>><br>
> >>> c0c0c0c0, abcdefed<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: 19017. N address=0x7fb23683bfc8<br>
> >>><br>
> >>> frag=0x7fb23683bf98 size=24 used=1<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached:<br>
> >>> ../../parser/../ut.h: pkg_str_dup(733)<br>
> >>><br>
> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check=<br>
> >>><br>
> >>> c0c0c0c0, abcdefed<br>
> >>><br>
> >>> The one related to pkg_str_dup() should be fixed by the commit:<br>
> >>><br>
> >>> -<br>
> >>> <a href="http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6faf" target="_blank">http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6faf</a><br>
> >>> 12653c1db9f011b1826061824c831bda3f58<br>
> >>><br>
> >>> The other one is related to mcd_malloc(), which I guess it is due to the<br>
> >>> function that returns the value from memchache, memcached_get() used in<br>
> >>><br>
> >>> pv_get_mcd_value_helper() -- the returned object has to be freed by<br>
> >>><br>
> >>> calling code, according to:<br>
> >>><br>
> >>> - <a href="http://docs.libmemcached.org/memcached_get.html" target="_blank">http://docs.libmemcached.org/memcached_get.html</a><br>
> >>><br>
> >>> Since the libmemcached was initialized with wrappers around pkg<br>
> >>> malloc/free, I expect the respective free function has to be used to<br>
> >>> free<br>
> >>> the result.<br>
> >>><br>
> >>> Can any of memcached devs check my commit and investigate further the<br>
> >>> second leak?<br>
> >>><br>
> >>> Cheers,<br>
> >>> Daniel<br>
> >>><br>
> >>> --<br>
> >>> Daniel-Constantin Mierla - <a href="http://www.asipto.com" target="_blank">http://www.asipto.com</a><br>
> >>> <a href="http://twitter.com/#!/miconda" target="_blank">http://twitter.com/#!/miconda</a> - <a href="http://www.linkedin.com/in/miconda" target="_blank">http://www.linkedin.com/in/miconda</a><br>
> >>> Kamailio Advanced Trainings - Berlin, Nov 25-28; Miami, Nov 18-20, 2013<br>
> >>><br>
> >>> - more details about Kamailio trainings at <a href="http://www.asipto.com" target="_blank">http://www.asipto.com</a> -<br>
> >>><br>
> >>> _______________________________________________<br>
> >>> sr-dev mailing list<br>
> >>> <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> >>> <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
> >>><br>
> >>> <a href="http://www.sipcentric.com" target="_blank">www.sipcentric.com</a><br>
> >>><br>
</div></div>> >>> Follow us on twitter @sipcentric <<a href="http://twitter.com/sipcentric" target="_blank">http://twitter.com/sipcentric</a>><br>
<div class="im">> >>><br>
> >>> Sipcentric Ltd. Company registered in England & Wales no. 7365592.<br>
> >>> Registered office: Unit 10 iBIC, Birmingham Science Park, Holt Court<br>
> >>> South, Birmingham B7 4EJ.<br>
> >>><br>
> >>> _______________________________________________<br>
> >>> sr-dev mailing list<br>
> >>> <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> >>> <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
> >>><br>
> >>><br>
> >>><br>
> >>> _______________________________________________<br>
> >>> sr-dev mailing list<br>
> >>> <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> >>> <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
> >><br>
> >> <a href="http://www.sipcentric.com" target="_blank">www.sipcentric.com</a><br>
> >><br>
</div>> >> Follow us on twitter @sipcentric <<a href="http://twitter.com/sipcentric" target="_blank">http://twitter.com/sipcentric</a>><br>
<div class="im">> >><br>
> >> Sipcentric Ltd. Company registered in England & Wales no. 7365592.<br>
> >> Registered office: Unit 10 iBIC, Birmingham Science Park, Holt Court<br>
> >> South, Birmingham B7 4EJ.<br>
> >><br>
> >> _______________________________________________<br>
> >> sr-dev mailing list<br>
> >> <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> >> <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
> ><br>
> > _______________________________________________<br>
> > sr-dev mailing<br>
> > listsr-dev@lists.sip-router.orghttp://<a href="http://lists.sip-router.org/cgi-bin/mailma" target="_blank">lists.sip-router.org/cgi-bin/mailma</a><br>
> > n/listinfo/sr-dev<br>
> ><br>
> ><br>
</div>> > --<br>
> > Daniel-Constantin Mierla -<br>
> > http://www.asipto.comhttp://<a href="http://twitter.com/#!/miconda" target="_blank">twitter.com/#!/miconda</a> -<br>
<div class="HOEnZb"><div class="h5">> > <a href="http://www.linkedin.com/in/miconda" target="_blank">http://www.linkedin.com/in/miconda</a> Kamailio Advanced Trainings - Berlin,<br>
> > Nov 25-28; Miami, Nov 18-20, 2013><br>
> > - more details about Kamailio trainings at <a href="http://www.asipto.com" target="_blank">http://www.asipto.com</a> -<br>
> ><br>
> > _______________________________________________<br>
> > sr-dev mailing list<br>
> > <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> > <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><font face="arial, helvetica, sans-serif"><b><font>Charles Chance</font></b><br><font>Managing Director</font></font><br><div><font face="arial, helvetica, sans-serif"><font><br>
</font></font></div><div><font face="arial, helvetica, sans-serif"><font>t. 0121 285 4400 m. 07932 063 891</font></font></div></div>
</div>
<br>
<font face="Helvetica, Arial, sans-serif"><font size="2"><span style="font-size:10pt"><a href="http://www.sipcentric.com/" title="blocked::http://www.sipcentric.com/" target="_blank">www.sipcentric.com</a><br>
<br>
Follow us on twitter <a href="http://twitter.com/sipcentric" title="blocked::http://twitter.com/sipcentric" target="_blank">@sipcentric</a><br>
<br>
<font color="gray">Sipcentric Ltd.
Company registered in England & Wales no. 7365592.</font> <font color="gray">Registered
office: Unit 10 iBIC, Birmingham Science Park, Holt Court South, Birmingham B7 4EJ.</font></span></font></font>