2 Oct
2013
2 Oct
'13
10:22 p.m.
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=6faf126…
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 -
3 Oct
3 Oct
8:27 p.m.
I can take a look this evening. Assuming nobody has already started?
Best,
Charles
On 2 Oct 2013 20:23, "Daniel-Constantin Mierla" <miconda(a)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=
**6faf12653c1db9f011b1826061824c**831bda3f58<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<http://docs.libmemcach…
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<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(a)lists.sip-router.org
http://lists.sip-router.org/**cgi-bin/mailman/listinfo/sr-**dev<http://l…
--
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.
8:56 p.m.
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(a)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(a)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=**6faf12653c1db9f011b1826061824c**831bda3f58<http://git.sip-rou…
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<http://docs.libmemcach…
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<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(a)lists.sip-router.org
http://lists.sip-router.org/**cgi-bin/mailman/listinfo/sr-**dev<http://l…
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(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
11:30 p.m.
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(a)sipcentric.com>
To: Daniel-Constantin Mierla <miconda(a)gmail.com>
Cc: sr-dev <sr-dev(a)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(a)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=6faf126…
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(a)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(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
4 Oct
4 Oct
2 a.m.
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(a)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(a)sipcentric.com>
*To:* Daniel-Constantin Mierla <miconda(a)gmail.com>
*Cc:* sr-dev <sr-dev(a)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(a)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=
**6faf12653c1db9f011b1826061824c**831bda3f58<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<http://docs.libmemcach…
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<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(a)lists.sip-router.org
http://lists.sip-router.org/**cgi-bin/mailman/listinfo/sr-**dev<http://l…
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(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
_______________________________________________
sr-dev mailing list
sr-dev(a)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.
10:34 a.m.
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(a)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(a)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(a)sipcentric.com>
*To:* Daniel-Constantin Mierla <miconda(a)gmail.com>
*Cc:* sr-dev <sr-dev(a)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(a)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=**6faf12653c1db9f011b1826061824c**831bda3f58<http://git.sip-rou…
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<http://docs.libmemcach…
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<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(a)lists.sip-router.org
http://lists.sip-router.org/**cgi-bin/mailman/listinfo/sr-**dev<http://l…
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(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
_______________________________________________
sr-dev mailing list
sr-dev(a)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(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
11:12 a.m.
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(a)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(a)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(a)sipcentric.com
<mailto:charles.chance@sipcentric.com>>
*To:* Daniel-Constantin Mierla <miconda(a)gmail.com
<mailto:miconda@gmail.com>>
*Cc:* sr-dev <sr-dev(a)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(a)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=6faf126…
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(a)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(a)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(a)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(a)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(a)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 -
11:40 a.m.
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(a)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(a)sipcentric.com> wrote:
--
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.
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(a)yahoo.com> wrote:
_______________________________________________
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(a)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(a)sipcentric.com>
*To:* Daniel-Constantin Mierla <miconda(a)gmail.com>
*Cc:* sr-dev <sr-dev(a)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(a)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=6faf126…
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(a)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(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
_______________________________________________
sr-dev mailing list
sr-dev(a)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(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
1:40 p.m.
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(a)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(a)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(a)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(a)sipcentric.com>
>>>
>>> *To:* Daniel-Constantin Mierla <miconda(a)gmail.com>
>>> *Cc:* sr-dev <sr-dev(a)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(a)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(a)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(a)lists.sip-router.org
>>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>>
>>>
>>>
>>> _______________________________________________
>>> sr-dev mailing list
>>> sr-dev(a)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(a)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(a)lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
2:16 p.m.
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(a)1und1.de>wrote;wrote:
Am Freitag, 4. Oktober 2013, 09:40:17 schrieb Charles
Chance:
--
*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.
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(a)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(a)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(a)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(a)sipcentric.com>
>>>
>>> *To:* Daniel-Constantin Mierla <miconda(a)gmail.com>
>>> *Cc:* sr-dev <sr-dev(a)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(a)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(a)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(a)lists.sip-router.org
>>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>>
>>>
>>>
>>> _______________________________________________
>>> sr-dev mailing list
>>> sr-dev(a)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(a)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(a)lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
3:43 p.m.
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
4:32 p.m.
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(a)1und1.de>wrote;wrote:
Am Freitag, 4. Oktober 2013, 12:16:32 schrieb Charles
Chance:
--
*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.
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
5:04 p.m.
Hello,
On 10/4/13 2:43 PM, Henning Westerholt wrote:
Am Freitag, 4. Oktober 2013, 12:16:32 schrieb Charles
Chance:
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
--
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 -
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.
4071
days inactive
4073
days old
12 comments
5 participants
participants (5)
-
Charles Chance -
Daniel-Constantin Mierla -
Dragos Oancea -
Federico Cabiddu -
Henning Westerholt