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=6faf1265...
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
I can take a look this evening. Assuming nobody has already started?
Best,
Charles On 2 Oct 2013 20:23, "Daniel-Constantin Mierla" miconda@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:
**6faf12653c1db9f011b1826061824c**831bda3f58http://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:
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/**micondahttp://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@lists.sip-router.org http://lists.sip-router.org/**cgi-bin/mailman/listinfo/sr-**devhttp://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
Hi Charles, I've been looking at this today. Since we were in a hurry to understand the issue I applied a quick fix (attached) just to test and it solved our current issue. If you want I can work more on it in order to have a better and cleaner solution useful in the other cases where pv_get_mcd_value_helper is called.
Best regards,
Federico Cabiddu
On Thu, Oct 3, 2013 at 7:27 PM, Charles Chance < charles.chance@sipcentric.com> wrote:
I can take a look this evening. Assuming nobody has already started?
Best,
Charles On 2 Oct 2013 20:23, "Daniel-Constantin Mierla" miconda@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:
commit;h=**6faf12653c1db9f011b1826061824c**831bda3f58http://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:
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/**micondahttp://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@lists.sip-router.org http://lists.sip-router.org/**cgi-bin/mailman/listinfo/sr-**devhttp://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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
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@sipcentric.com To: Daniel-Constantin Mierla miconda@gmail.com Cc: sr-dev sr-dev@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@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:
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:
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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
www.sipcentric.com
Follow us on twitter @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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
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@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@sipcentric.com *To:* Daniel-Constantin Mierla miconda@gmail.com *Cc:* sr-dev sr-dev@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@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:
**6faf12653c1db9f011b1826061824c**831bda3f58http://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:
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/**micondahttp://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@lists.sip-router.org http://lists.sip-router.org/**cgi-bin/mailman/listinfo/sr-**devhttp://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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
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@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@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@sipcentric.com *To:* Daniel-Constantin Mierla miconda@gmail.com *Cc:* sr-dev sr-dev@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@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:
commit;h=**6faf12653c1db9f011b1826061824c**831bda3f58http://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:
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/**micondahttp://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@lists.sip-router.org http://lists.sip-router.org/**cgi-bin/mailman/listinfo/sr-**devhttp://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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
sr-dev mailing list sr-dev@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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
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@sipcentric.com mailto:charles.chance@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@yahoo.com <mailto:droancea@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@sipcentric.com <mailto:charles.chance@sipcentric.com>> *To:* Daniel-Constantin Mierla <miconda@gmail.com <mailto:miconda@gmail.com>> *Cc:* sr-dev <sr-dev@lists.sip-router.org <mailto:sr-dev@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@gmail.com <mailto:miconda@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@lists.sip-router.org <mailto:sr-dev@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@lists.sip-router.org <mailto:sr-dev@lists.sip-router.org> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev _______________________________________________ sr-dev mailing list sr-dev@lists.sip-router.org <mailto:sr-dev@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@lists.sip-router.org <mailto:sr-dev@lists.sip-router.org> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
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@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@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@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@sipcentric.com *To:* Daniel-Constantin Mierla miconda@gmail.com *Cc:* sr-dev sr-dev@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@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=6faf1265...
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:
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@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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
sr-dev mailing list sr-dev@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@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/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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
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@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@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@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@sipcentric.com
*To:* Daniel-Constantin Mierla miconda@gmail.com *Cc:* sr-dev sr-dev@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@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:
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@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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
sr-dev mailing list sr-dev@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@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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
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@1und1.dewrote:
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@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@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@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@sipcentric.com
*To:* Daniel-Constantin Mierla miconda@gmail.com *Cc:* sr-dev sr-dev@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@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:
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@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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
sr-dev mailing list sr-dev@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@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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
Am Freitag, 4. Oktober 2013, 12:16:32 schrieb Charles Chance:
Sorry hit reply only, first time.
Would you like me still to update to use mcd_free or leave it with you?
Hi Charles,
just commit the patch, looks good for me. The only small thing I noticed is that you changed the indention in the variable definition in pv_set_mcd_expire from tabs to space, would be great if you could adapt this.
With regards to the other wrapper (mcd_free) that Daniel mentioned, this is only used as callback function for the library. I'll clarify this in the comments that this is the purpose of this code.
Regards,
Henning Westerholt
Hi Henning,
The only small thing I noticed is that you changed the indention in the
variable definition in pv_set_mcd_expire from tabs to space
Unintentional, sorry, not sure how that happened :/
just commit the patch, looks good for me.
Thanks, will do.
Best,
Charles
On 4 October 2013 13:43, Henning Westerholt henning.westerholt@1und1.dewrote:
Am Freitag, 4. Oktober 2013, 12:16:32 schrieb Charles Chance:
Sorry hit reply only, first time.
Would you like me still to update to use mcd_free or leave it with you?
Hi Charles,
just commit the patch, looks good for me. The only small thing I noticed is that you changed the indention in the variable definition in pv_set_mcd_expire from tabs to space, would be great if you could adapt this.
With regards to the other wrapper (mcd_free) that Daniel mentioned, this is only used as callback function for the library. I'll clarify this in the comments that this is the purpose of this code.
Regards,
Henning Westerholt
Hello,
On 10/4/13 2:43 PM, Henning Westerholt wrote:
Am Freitag, 4. Oktober 2013, 12:16:32 schrieb Charles Chance:
Sorry hit reply only, first time.
Would you like me still to update to use mcd_free or leave it with you?
Hi Charles,
just commit the patch, looks good for me. The only small thing I noticed is that you changed the indention in the variable definition in pv_set_mcd_expire from tabs to space, would be great if you could adapt this.
With regards to the other wrapper (mcd_free) that Daniel mentioned, this is only used as callback function for the library. I'll clarify this in the comments that this is the purpose of this code.
my comment was related to the fact that I haven't seen an explicit pkg_malloc() for the returned value from the libmacached function. I assumed it is going to use the callback mcd_malloc(), based on libmemcached api docs, which say that the result has to be released by caller.
From that perspective, as the result is allocated with mcd_malloc(), it looks more consistent if is going to be freed with mcd_free(). A change in mcd_malloc() would naturally trigger an appropriate change in mcd_free() (e.g., using shm_malloc() instead of pkg_malloc()), while now if someone changes mcd_malloc() and mcd_free() would have to know that has to change the pkg_free() in the other part of the code.
So, in short, using pkg_free() now solves the issue, it is ok, just that doesn't look coherent.
Cheers, Daniel
-
Charles Chance -
Daniel-Constantin Mierla -
Dragos Oancea -
Federico Cabiddu -
Henning Westerholt