[sr-dev] memory leak in memcached module
Charles Chance
charles.chance at sipcentric.com
Fri Oct 4 13:16:32 CEST 2013
Hello Henning,
Sorry hit reply only, first time.
Would you like me still to update to use mcd_free or leave it with you?
Best,
Charles
On 4 October 2013 11:40, Henning Westerholt <henning.westerholt at 1und1.de>wrote:
> Am Freitag, 4. Oktober 2013, 09:40:17 schrieb Charles Chance:
> > Yes, agreed - I'll update it today. Henning Westerholt was original
> author
> > if he'd like to review before I commit?
>
> Hello Charles,
>
> I'll take a look today to it as well, thank you.
>
> Regards,
>
> Henning
>
> > 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=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 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/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 at lists.sip-router.org
> > > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>
--
*Charles Chance*
Managing Director
t. 0121 285 4400 m. 07932 063 891
--
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/5186ce7c/attachment-0001.html>
More information about the sr-dev
mailing list