[sr-dev] memory leak in memcached module
Charles Chance
charles.chance at sipcentric.com
Fri Oct 4 10:40:17 CEST 2013
Hi Daniel,
Yes, agreed - I'll update it today. Henning Westerholt was original author
if he'd like to review before I commit?
Best regards,
Charles
On 4 Oct 2013 09:12, "Daniel-Constantin Mierla" <miconda at gmail.com> 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 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> 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>
>>> *To:* Daniel-Constantin Mierla <miconda at gmail.com>
>>> *Cc:* sr-dev <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>
>>> 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://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 at 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 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
>>>
>>>
>>
>>
>>
>> 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
>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>
>>
>
>
> _______________________________________________
> sr-dev mailing listsr-dev at lists.sip-router.orghttp://lists.sip-router.org/cgi-bin/mailman/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 at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20131004/84fe52d4/attachment-0001.html>
More information about the sr-dev
mailing list