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