<div dir="ltr">Hi Federico/Dragos,<div><br></div><div>Thank you both for your input. I have placed the call to (pkg_)free into a separate function and called it where necessary from each of the other functions.</div><div><br>
</div><div>I haven't had chance to test yet - if you have time, please apply the attached diff and let me know if the leak is fixed and I will commit to master. Otherwise, I will test myself over the weekend.</div><div>
<br></div><div>Thanks again,</div><div><br></div><div>Charles</div><div><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On 3 October 2013 21:30, Dragos Oancea <span dir="ltr"><<a href="mailto:droancea@yahoo.com" target="_blank">droancea@yahoo.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div style="font-size:10pt;font-family:arial,helvetica,sans-serif"><div><span>Hi Charles,</span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<span><br></span></div><div style="font-size:13px;font-family:arial,helvetica,sans-serif;background-color:transparent;font-style:normal"><span style>In the function where Daniel just made the fix for the memory leak (</span><span style="font-family:monospace;font-size:12px;white-space:pre-wrap">int pv_get_mcd_value() </span><span style="background-color:transparent">) , just before existing it with 0 , we added something like the following :</span></div>
<div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><span style="background-color:transparent"><br></span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<span style="background-color:transparent">if (mcd_memory) {</span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
pkg_free(return_value); </div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<span style="background-color:transparent">} else {</span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><span style="background-color:transparent"><br>
</span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><span style="background-color:transparent"><span style="white-space:pre-wrap"> </span>free(return_value); </span></div>
<div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><span style="background-color:transparent">}</span></div><div><br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
It looks like it does not leak anymore, but please-double check if we are free-ing it in the right place. </div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
Regards,</div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">Dragos</div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<br></div> <div style="font-family:arial,helvetica,sans-serif;font-size:10pt"> <div style="font-family:'times new roman','new york',times,serif;font-size:12pt"> <div dir="ltr"> <hr size="1"> <font face="Arial"> <b><span style="font-weight:bold">From:</span></b> Charles Chance <<a href="mailto:charles.chance@sipcentric.com" target="_blank">charles.chance@sipcentric.com</a>><br>
<b><span style="font-weight:bold">To:</span></b> Daniel-Constantin Mierla <<a href="mailto:miconda@gmail.com" target="_blank">miconda@gmail.com</a>> <br><b><span style="font-weight:bold">Cc:</span></b> sr-dev <<a href="mailto:sr-dev@lists.sip-router.org" target="_blank">sr-dev@lists.sip-router.org</a>> <br>
<b><span style="font-weight:bold">Sent:</span></b> Thursday, October 3, 2013 7:27 PM<br> <b><span style="font-weight:bold">Subject:</span></b> Re: [sr-dev] memory leak in memcached module<br> </font> </div> <div><br><div>
<div><div class="h5"><div dir="ltr">I can take a look this evening. Assuming nobody has already started?</div>
<div dir="ltr">Best,</div>
<div dir="ltr">Charles<br>
</div>
<div>On 2 Oct 2013 20:23, "Daniel-Constantin Mierla" <<a rel="nofollow" href="mailto:miconda@gmail.com" target="_blank">miconda@gmail.com</a>> wrote:<br><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello,<br>
<br>
there is (still) a memory leak in memcached module, discovered on a report by Dragos Oancea.<br>
<br>
The pkg usage logs are like:<br>
<br>
0(24328) NOTICE: qm_status: 19010. N address=0x7fb23683bc98 frag=0x7fb23683bc68 size=8 used=1<br>
0(24328) NOTICE: qm_status: alloc'd from memcached: ../../parser/../ut.h: pkg_str_dup(733)<br>
0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= c0c0c0c0, abcdefed<br>
0(24328) NOTICE: qm_status: 19011. N address=0x7fb23683bd00 frag=0x7fb23683bcd0 size=48 used=1<br>
0(24328) NOTICE: qm_status: alloc'd from memcached: memcached.c: mcd_malloc(127)<br>
0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= c0c0c0c0, abcdefed<br>
0(24328) NOTICE: qm_status: 19012. N address=0x7fb23683bd90 frag=0x7fb23683bd60 size=8 used=1<br>
0(24328) NOTICE: qm_status: alloc'd from memcached: ../../parser/../ut.h: pkg_str_dup(733)<br>
0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= c0c0c0c0, abcdefed<br>
0(24328) NOTICE: qm_status: 19013. N address=0x7fb23683bdf8 frag=0x7fb23683bdc8 size=48 used=1<br>
0(24328) NOTICE: qm_status: alloc'd from memcached: memcached.c: mcd_malloc(127)<br>
0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= c0c0c0c0, abcdefed<br>
0(24328) NOTICE: qm_status: 19014. N address=0x7fb23683be88 frag=0x7fb23683be58 size=8 used=1<br>
0(24328) NOTICE: qm_status: alloc'd from memcached: memcached.c: mcd_malloc(127)<br>
0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= c0c0c0c0, abcdefed<br>
0(24328) NOTICE: qm_status: 19015. N address=0x7fb23683bef0 frag=0x7fb23683bec0 size=16 used=1<br>
0(24328) NOTICE: qm_status: alloc'd from memcached: ../../parser/../ut.h: pkg_str_dup(733)<br>
0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= c0c0c0c0, abcdefed<br>
0(24328) NOTICE: qm_status: 19016. N address=0x7fb23683bf60 frag=0x7fb23683bf30 size=8 used=1<br>
0(24328) NOTICE: qm_status: alloc'd from memcached: memcached.c: mcd_malloc(127)<br>
0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= c0c0c0c0, abcdefed<br>
0(24328) NOTICE: qm_status: 19017. N address=0x7fb23683bfc8 frag=0x7fb23683bf98 size=24 used=1<br>
0(24328) NOTICE: qm_status: alloc'd from memcached: ../../parser/../ut.h: pkg_str_dup(733)<br>
0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= c0c0c0c0, abcdefed<br>
<br>
The one related to pkg_str_dup() should be fixed by the commit:<br>
<br>
- <a rel="nofollow" href="http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6faf12653c1db9f011b1826061824c831bda3f58" target="_blank">http://git.sip-router.org/cgi-<u></u>bin/gitweb.cgi/sip-router/?a=<u></u>commit;h=<u></u>6faf12653c1db9f011b1826061824c<u></u>831bda3f58</a><br>
<br>
The other one is related to mcd_malloc(), which I guess it is due to the function that returns the value from memchache, memcached_get() used in pv_get_mcd_value_helper() -- the returned object has to be freed by calling code, according to:<br>
<br>
- <a rel="nofollow" href="http://docs.libmemcached.org/memcached_get.html" target="_blank">http://docs.libmemcached.org/<u></u>memcached_get.html</a><br>
<br>
Since the libmemcached was initialized with wrappers around pkg malloc/free, I expect the respective free function has to be used to free the result.<br>
<br>
Can any of memcached devs check my commit and investigate further the second leak?<br>
<br>
Cheers,<br>
Daniel<br>
<br>
-- <br>
Daniel-Constantin Mierla - <a rel="nofollow" href="http://www.asipto.com/" target="_blank">http://www.asipto.com</a><br>
<a rel="nofollow" href="http://twitter.com/#!/miconda" target="_blank">http://twitter.com/#!/miconda</a> - <a rel="nofollow" href="http://www.linkedin.com/in/miconda" target="_blank">http://www.linkedin.com/in/<u></u>miconda</a><br>
Kamailio Advanced Trainings - Berlin, Nov 25-28; Miami, Nov 18-20, 2013<br>
- more details about Kamailio trainings at <a rel="nofollow" href="http://www.asipto.com/" target="_blank">http://www.asipto.com</a> -<br>
<br>
<br>
______________________________<u></u>_________________<br>
sr-dev mailing list<br>
<a rel="nofollow" href="mailto:sr-dev@lists.sip-router.org" target="_blank">sr-dev@lists.sip-router.org</a><br>
<a rel="nofollow" href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/<u></u>cgi-bin/mailman/listinfo/sr-<u></u>dev</a><br>
</blockquote></div>
<br>
</div></div><div class="im"><font face="Helvetica, Arial, sans-serif"><font><span style="font-size:10pt"><a rel="nofollow" href="http://www.sipcentric.com/" title="blocked::http://www.sipcentric.com/" target="_blank">www.sipcentric.com</a><br>
<br>
Follow us on twitter <a rel="nofollow" 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></div></div><br><div class="im">_______________________________________________<br>sr-dev mailing list<br>
<a href="mailto:sr-dev@lists.sip-router.org" target="_blank">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></div></div> </div> </div> </div></div><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></blockquote></div><br><br clear="all"><div><br></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>