Re: [sr-dev] memory leak in memcached module

Yes, agreed - I'll update it today. Henning Westerholt was original author if he'd like to review before I commit?

On 4 Oct 2013 09:12, "Daniel-Constantin Mierla" &lt;miconda(a)gmail.com&gt; wrote: > Hello, > > for consistency, it might be good that mcd_free() is used instead of > pkg_free when mcd_memory is set, because it is supposed to be the pair of > mcd_malloc() which is used to allocate the memory. mcd_free() is a wrapper > around pkg_free(), so works like current patch, but it is safer for future > changes to mcd_*() functions. > > These are remarks just looking at the current code, I am not familiar with > memcached lib and api. > > Cheers, > Daniel > > On 10/4/13 9:34 AM, Federico Cabiddu wrote: > > Hi Charles, > I just tried the patch running the test I used to reproduce the leak. > It's ok, the memory debug shows that the memory is correctly released. > I tested with memory parameter set to 0 and to 1. > Thank you. > > Best regards, > > Federico > > On Fri, Oct 4, 2013 at 1:00 AM, Charles Chance < > > charles.chance(a)sipcentric.com&gt; wrote: >> Hi Federico/Dragos, >> >> 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. >> >> 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. >> >> Thanks again, >> >> Charles >> >> On 3 October 2013 21:30, Dragos Oancea &lt;droancea(a)yahoo.com&gt; wrote: >>> Hi Charles, >>> >>> In the function where Daniel just made the fix for the memory leak (int >>> pv_get_mcd_value()>>> >>> ) , just before existing it with 0 , we added something like the >>> >>> following : >>> if (mcd_memory) { >>> >>> pkg_free(return_value); >>> >>> } else { >>> >>> free(return_value); >>> >>> } >>> >>> It looks like it does not leak anymore, but please-double check if we >>> >>> are free-ing it in the right place. >>> >>> Regards, >>> >>> Dragos >>> >>> ------------------------------ >>> >>> *From:* Charles Chance &lt;charles.chance(a)sipcentric.com&gt; >>> >>> *To:* Daniel-Constantin Mierla &lt;miconda(a)gmail.com&gt; >>> *Cc:* sr-dev &lt;sr-dev(a)lists.sip-router.org&gt; >>> *Sent:* Thursday, October 3, 2013 7:27 PM >>> *Subject:* Re: [sr-dev] memory leak in memcached module >>> >>> I can take a look this evening. Assuming nobody has already started? >>> >>> Best, >>> Charles >>> >>> On 2 Oct 2013 20:23, "Daniel-Constantin Mierla" &lt;miconda(a)gmail.com&gt; >>> >>> wrote: >>> >>> Hello, >>> >>> there is (still) a memory leak in memcached module, discovered on a >>> report by Dragos Oancea. >>> >>> The pkg usage logs are like: >>> >>> 0(24328) NOTICE: qm_status: 19010. N address=0x7fb23683bc98 >>> frag=0x7fb23683bc68 size=8 used=1 >>> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached: >>> ../../parser/../ut.h: pkg_str_dup(733) >>> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= >>> >>> c0c0c0c0, abcdefed >>> >>> 0(24328) NOTICE: qm_status: 19011. N address=0x7fb23683bd00 >>> >>> frag=0x7fb23683bcd0 size=48 used=1 >>> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached: >>> memcached.c: mcd_malloc(127) >>> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= >>> >>> c0c0c0c0, abcdefed >>> >>> 0(24328) NOTICE: qm_status: 19012. N address=0x7fb23683bd90 >>> >>> frag=0x7fb23683bd60 size=8 used=1 >>> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached: >>> ../../parser/../ut.h: pkg_str_dup(733) >>> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= >>> >>> c0c0c0c0, abcdefed >>> >>> 0(24328) NOTICE: qm_status: 19013. N address=0x7fb23683bdf8 >>> >>> frag=0x7fb23683bdc8 size=48 used=1 >>> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached: >>> memcached.c: mcd_malloc(127) >>> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= >>> >>> c0c0c0c0, abcdefed >>> >>> 0(24328) NOTICE: qm_status: 19014. N address=0x7fb23683be88 >>> >>> frag=0x7fb23683be58 size=8 used=1 >>> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached: >>> memcached.c: mcd_malloc(127) >>> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= >>> >>> c0c0c0c0, abcdefed >>> >>> 0(24328) NOTICE: qm_status: 19015. N address=0x7fb23683bef0 >>> >>> frag=0x7fb23683bec0 size=16 used=1 >>> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached: >>> ../../parser/../ut.h: pkg_str_dup(733) >>> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= >>> >>> c0c0c0c0, abcdefed >>> >>> 0(24328) NOTICE: qm_status: 19016. N address=0x7fb23683bf60 >>> >>> frag=0x7fb23683bf30 size=8 used=1 >>> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached: >>> memcached.c: mcd_malloc(127) >>> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= >>> >>> c0c0c0c0, abcdefed >>> >>> 0(24328) NOTICE: qm_status: 19017. N address=0x7fb23683bfc8 >>> >>> frag=0x7fb23683bf98 size=24 used=1 >>> >>> 0(24328) NOTICE: qm_status: alloc'd from memcached: >>> ../../parser/../ut.h: pkg_str_dup(733) >>> >>> 0(24328) NOTICE: qm_status: start check=f0f0f0f0, end check= >>> >>> c0c0c0c0, abcdefed >>> >>> The one related to pkg_str_dup() should be fixed by the commit: >>> >>> - >>> http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6faf >>> 12653c1db9f011b1826061824c831bda3f58 >>> >>> 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: >>> >>> - http://docs.libmemcached.org/memcached_get.html >>> >>> 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. >>> >>> Can any of memcached devs check my commit and investigate further the >>> second leak? >>> >>> Cheers, >>> Daniel >>> >>> -- >>> Daniel-Constantin Mierla - http://www.asipto.com >>> http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda >>> Kamailio Advanced Trainings - Berlin, Nov 25-28; Miami, Nov 18-20, 2013 >>> >>> - more details about Kamailio trainings at http://www.asipto.com - >>> >>> _______________________________________________ >>> sr-dev mailing list >>> sr-dev(a)lists.sip-router.org >>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev >>> >>> www.sipcentric.com >>> >>> Follow us on twitter @sipcentric <http://twitter.com/sipcentric> >>> >>> Sipcentric Ltd. Company registered in England & Wales no. 7365592. >>> Registered office: Unit 10 iBIC, Birmingham Science Park, Holt Court >>> South, Birmingham B7 4EJ. >>> >>> _______________________________________________ >>> sr-dev mailing list >>> sr-dev(a)lists.sip-router.org >>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev >>> >>> >>> >>> _______________________________________________ >>> sr-dev mailing list >>> sr-dev(a)lists.sip-router.org >>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev >> >> www.sipcentric.com >> >> Follow us on twitter @sipcentric <http://twitter.com/sipcentric> >> >> Sipcentric Ltd. Company registered in England & Wales no. 7365592. >> Registered office: Unit 10 iBIC, Birmingham Science Park, Holt Court >> South, Birmingham B7 4EJ. >> >> _______________________________________________ >> sr-dev mailing list >> sr-dev(a)lists.sip-router.org >> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev > > _______________________________________________ > sr-dev mailing > listsr-dev@lists.sip-router.orghttp://lists.sip-router.org/cgi-bin/mailma > n/listinfo/sr-dev > > > -- > Daniel-Constantin Mierla - > http://www.asipto.comhttp://twitter.com/#!/miconda - > http://www.linkedin.com/in/miconda Kamailio Advanced Trainings - Berlin, > Nov 25-28; Miami, Nov 18-20, 2013> > - more details about Kamailio trainings at http://www.asipto.com - > > _______________________________________________ > sr-dev mailing list > sr-dev(a)lists.sip-router.org > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev