[sr-dev] memory leak in memcached module

Daniel-Constantin Mierla miconda at gmail.com
Fri Oct 4 10:12:17 CEST 2013


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 at sipcentric.com <mailto:charles.chance at sipcentric.com>> 
> 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 <droancea at yahoo.com
>     <mailto:droancea at yahoo.com>> 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 <charles.chance at sipcentric.com
>         <mailto:charles.chance at sipcentric.com>>
>         *To:* Daniel-Constantin Mierla <miconda at gmail.com
>         <mailto:miconda at gmail.com>>
>         *Cc:* sr-dev <sr-dev at lists.sip-router.org
>         <mailto:sr-dev at lists.sip-router.org>>
>         *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"
>         <miconda at gmail.com <mailto:miconda at gmail.com>> 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=6faf12653c1db9f011b1826061824c831bda3f58
>
>             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://www.asipto.com/>
>             http://twitter.com/#!/miconda
>             <http://twitter.com/#%21/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 <http://www.asipto.com/> -
>
>
>             _______________________________________________
>             sr-dev mailing list
>             sr-dev at lists.sip-router.org
>             <mailto:sr-dev at lists.sip-router.org>
>             http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>
>         www.sipcentric.com <http://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 at lists.sip-router.org <mailto:sr-dev at lists.sip-router.org>
>         http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>
>
>         _______________________________________________
>         sr-dev mailing list
>         sr-dev at lists.sip-router.org <mailto:sr-dev at lists.sip-router.org>
>         http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>
>
>
>
>     www.sipcentric.com <http://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 at lists.sip-router.org <mailto:sr-dev at lists.sip-router.org>
>     http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>
>
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

-- 
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 -

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20131004/53356153/attachment-0001.html>


More information about the sr-dev mailing list