Hi Daniel, I tried the patch, it works partially. There are stil memory leak. Based on your patch, I did find different places need the same kind of patch both in send_subscribe,c and in pua,c
Please find the modified version of these files.
Following are the differences: # diff -u send_subscribe.c.orig send_subscribe.c --- send_subscribe.c.orig 2011-12-27 13:31:06.000000000 +0100 +++ send_subscribe.c 2011-12-27 13:31:51.000000000 +0100 @@ -1151,7 +1151,6 @@ if (dbmode!=PUA_DB_ONLY) lock_release(&HashT->p_records[hash_code].lock); ret= -1; - pkg_free(td); goto done; } if (dbmode!=PUA_DB_ONLY)
# diff -u pua.c.orig pua.c --- pua.c.orig 2011-12-27 13:15:47.000000000 +0100 +++ pua.c 2011-12-27 13:26:33.000000000 +0100 @@ -673,106 +673,145 @@
int update_pua(ua_pres_t* p) { - str* str_hdr= NULL; - int expires; - int result; - uac_req_t uac_r; - - if(p->desired_expires== 0) - expires= 3600; - else - expires= p->desired_expires- (int)time(NULL); + str* str_hdr= NULL; + ua_pres_t* cb_param = NULL;
- if(p->watcher_uri== NULL) - { - str met= {"PUBLISH", 7}; - ua_pres_t* cb_param; - - str_hdr = publ_build_hdr(expires, get_event(p->event), NULL, - &p->etag, p->extra_headers, 0); - if(str_hdr == NULL) - { - LM_ERR("while building extra_headers\n"); - goto error; - } - LM_DBG("str_hdr:\n%.*s\n ", str_hdr->len, str_hdr->s); - - cb_param= build_uppubl_cbparam(p); - if(cb_param== NULL) - { - LM_ERR("while constructing publ callback param\n"); - goto error; - } - - set_uac_req(&uac_r, &met, str_hdr, 0, 0, TMCB_LOCAL_COMPLETED, - publ_cback_func, (void*)cb_param); - - result= tmb.t_request(&uac_r, - p->pres_uri, /* Request-URI */ - p->pres_uri, /* To */ - p->pres_uri, /* From */ - &outbound_proxy /* Outbound proxy*/ - ); - if(result< 0) - { - LM_ERR("in t_request function\n"); - shm_free(cb_param); - goto error; - } - - } - else - { - str met= {"SUBSCRIBE", 9}; - dlg_t* td= NULL; - ua_pres_t* cb_param= NULL; - - td= pua_build_dlg_t(p); - if(td== NULL) - { - LM_ERR("while building tm dlg_t structure"); - goto error; - }; - - str_hdr= subs_build_hdr(&p->contact, expires,p->event,p->extra_headers); - if(str_hdr== NULL || str_hdr->s== NULL) - { - LM_ERR("while building extra headers\n"); - pkg_free(td); - return -1; - } - cb_param= subs_cbparam_indlg(p, expires, REQ_ME); - if(cb_param== NULL) - { - LM_ERR("while constructing subs callback param\n"); - goto error; - - } - - set_uac_req(&uac_r, &met, str_hdr, 0, td, TMCB_LOCAL_COMPLETED, - subs_cback_func, (void*)cb_param); - - result= tmb.t_request_within(&uac_r); - if(result< 0) - { - LM_ERR("in t_request function\n"); - shm_free(cb_param); - pkg_free(td); - goto error; - } - - pkg_free(td); - td= NULL; - } - - pkg_free(str_hdr); - return 0; - -error: - if(str_hdr) - pkg_free(str_hdr); - return -1; + int expires; + int result = 0; + uac_req_t uac_r; + str met = {NULL, 0}; + int ret_code = 0; + dlg_t* td = NULL; + + + if(p->desired_expires== 0) + expires= default_expires; + else + expires= p->desired_expires- (int)time(NULL); + + if(p->watcher_uri== NULL) + { + + str_hdr = publ_build_hdr(expires, get_event(p->event), NULL, + &p->etag, p->extra_headers, 0); + + if(str_hdr == NULL) + { + LM_ERR("while building extra_headers\n"); + ret_code = -1; + goto done; + } + LM_DBG("str_hdr:\n%.*s\n ", str_hdr->len, str_hdr->s); + + cb_param= build_uppubl_cbparam(p); + if(cb_param== NULL) + { + LM_ERR("while constructing publ callback param\n"); + ret_code = -1; + goto done; + } + + met.s = (char*)pkg_malloc(8*sizeof(char)); + if(met.s == NULL) { + LM_ERR("no memory for met.s(PUBLISH)\n"); + ret_code = -1; + goto done; + } + memset(met.s, 0, 8); + memcpy(met.s, "PUBLISH", 7); + met.len = 7; + set_uac_req(&uac_r, &met, str_hdr, 0, 0, TMCB_LOCAL_COMPLETED, + publ_cback_func, (void*)cb_param); + + result= tmb.t_request(&uac_r, + p->pres_uri, /* Request-URI */ + p->pres_uri, /* To */ + p->pres_uri, /* From */ + &outbound_proxy /* Outbound proxy*/ + ); + if(result< 0) + { + LM_ERR("in t_request function\n"); + shm_free(cb_param); + cb_param = NULL; + ret_code = -1; + goto done; + } + } + else + { + td= pua_build_dlg_t(p); + if(td== NULL) + { + LM_ERR("while building tm dlg_t structure"); + ret_code = -1; + goto done; + }; + + str_hdr= subs_build_hdr(&p->contact, expires,p->event,p->extra_headers); + if(str_hdr== NULL || str_hdr->s== NULL) + { + if(p->event!=0) + LM_ERR("while building extra headers\n"); + + ret_code = -1; + goto done; + } + + cb_param= subs_cbparam_indlg(p, expires, REQ_ME); + if(cb_param== NULL) + { + LM_ERR("while constructing subs callback param\n"); + ret_code = -1; + goto done; + } + + met.s = (char*)pkg_malloc(10*sizeof(char)); + if(met.s == NULL) { + LM_ERR("no memory for met.s(SUBSCRIBE)\n"); + ret_code = -1; + goto done; + } + memset(met.s, 0, 10); + memcpy(met.s, "SUBSCRIBE", 9); + met.len = 9; + set_uac_req(&uac_r, &met, str_hdr, 0, td, TMCB_LOCAL_COMPLETED, + subs_cback_func, (void*)cb_param); + + result= tmb.t_request_within(&uac_r); + if(result< 0) + { + LM_ERR("in t_request function\n"); + ret_code = -1; + shm_free(cb_param); + cb_param = NULL; + goto done; + } + } + + +done: + if(td!=NULL) + { + if(td->route_set) + free_rr(&td->route_set); + + pkg_free(td); + td= NULL; + } + + if(met.s != NULL) { + pkg_free(met.s); + met.s = NULL; + met.len = 0; + } + + if(str_hdr != NULL) { + pkg_free(str_hdr); + str_hdr = NULL; + }
+ return ret_code; }
static void db_update(unsigned int ticks,void *param)
It seems a lot of change, but there are only a few lines changed, probably in the file I modify I have convert the TAB to Space and the diff does not recognize them. Another thing is I use dynamic memory for the str SUBSCRIBE and PUBLISH.
Another thing for the management of NOTIFY arrives before the 202/200 OK of SUBSCRIBE, in addition to the patches done previously, maybe the hash.c need to patch for the is_dialog function which is called for the NOTIFY in PUA_XMPP module:
# diff -u hash.c.orig hash.c --- hash.c.orig 2011-12-27 13:38:06.000000000 +0100 +++ hash.c 2011-12-27 13:38:38.000000000 +0100 @@ -487,10 +487,11 @@ hash_code= core_hash(dialog->pres_uri, dialog->watcher_uri, HASH_SIZE); lock_get(&HashT->p_records[hash_code].lock);
- if(get_dialog(dialog, hash_code)== NULL) - ret_code= -1; - else + if(get_dialog(dialog, hash_code) || get_temporary_dialog(dialog, hash_code)) ret_code= 0; + else + ret_code= -1; + lock_release(&HashT->p_records[hash_code].lock);
return ret_code;
Best Regards, Laura
On Fri, Dec 23, 2011 at 5:10 PM, laura testi lau.testi@gmail.com wrote:
Ok, I'll try it.
Thank you very much!
On Fri, Dec 23, 2011 at 1:20 PM, Daniel-Constantin Mierla miconda@gmail.com wrote:
Hello,
can you try with this patch:
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=1b3cfa60...
Cheers, Daniel
On 12/23/11 12:36 PM, Daniel-Constantin Mierla wrote:
Hello,
looks like a leak in a module that is storing record-routes and use them later, perhaps pua module, I will check it soon.
Cheers, Daniel
On 12/23/11 11:31 AM, laura testi wrote:
Hi Daniel, I just follow the instruction in the link you sent (http://www.asipto.com/pub/kamailio-devel-guide/#c04troubleshooting) to use gdb to print PKG fragments. When I got the error in PUA:
Dec 23 11:10:53 /.../sbin/kamailio[23276]: ERROR: pua [send_subscribe.c:158]: No memory left for size:439 Dec 23 11:10:53 /.../sbin/kamailio[23276]: ERROR: pua [pua.c:747]: while building tm dlg_t structure Dec 23 11:10:53 /.../sbin/kamailio[23276]: ERROR: pua [pua.c:652]: while updating record
The I run the command gdb /.../sbin/kamailio 23276 and write the following commands in the gdb:
set $i=0 set $a = mem_block->first_frag while($i<10000) if($i>2000) if($a->u.is_free==0) p *$a end end set $a = ((struct qm_frag*)((char*)($a)+sizeof(struct qm_frag)+((struct qm_frag*)$a)->size+sizeof(struct qm_frag_end))) set $i = $i + 1 end ...
after a while I got a lot of prints on the screen like these:
func = 0x5d7b00 "do_parse_rr_body", line = 74, check = 4042322160} $1348 = {size = 72, u = {nxt_free = 0x0, is_free = 0}, file = 0x5d76c9 "<core>: parser/parse_rr.c", func = 0x5d7b00 "do_parse_rr_body", line = 74, check = 4042322160} $1349 = {size = 72, u = {nxt_free = 0x0, is_free = 0}, file = 0x5d76c9 "<core>: parser/parse_rr.c", func = 0x5d7b00 "do_parse_rr_body", line = 74, check = 4042322160} $1350 = {size = 72, u = {nxt_free = 0x0, is_free = 0}, file = 0x5d76c9 "<core>: parser/parse_rr.c", func = 0x5d7b00 "do_parse_rr_body", line = 74, check = 4042322160} $1351 = {size = 104, u = {nxt_free = 0x0, is_free = 0}, file = 0x5d76c9 "<core>: parser/parse_rr.c", func = 0x5d7b00 "do_parse_rr_body", line = 74, check = 4042322160} $1352 = {size = 104, u = {nxt_free = 0x0, is_free = 0}, file = 0x5d76c9 "<core>: parser/parse_rr.c", func = 0x5d7b00 "do_parse_rr_body", line = 74, check = 4042322160} $1353 = {size = 72, u = {nxt_free = 0x0, is_free = 0}, file = 0x5d76c9 "<core>: parser/parse_rr.c", func = 0x5d7b00 "do_parse_rr_body", line = 74, check = 4042322160} $1354 = {size = 72, u = {nxt_free = 0x0, is_free = 0}, ---Type<return> to continue, or q<return> to quit---
...
But I don't understand if these are normal or something goes wrong....
Can you help
Best Regards, Laura
On Wed, Dec 21, 2011 at 12:18 PM, Daniel-Constantin Mierla miconda@gmail.com wrote:
Hello,
pkg.stats was added in 3.2.0, iirc. For 3.1, you can walk the packets in memory with gdb -- 3.1 has memory debug on, so you don't need to recompile (unless you turned it off).
Just attach to the pid of a sip worker (gdb /path/to/kamailio _pid_value_) and run the gdb script.
Cheers, Daniel
On 12/21/11 11:58 AM, laura testi wrote:
Hi Daniel, I try the sercmd for pkg memory but it return 500 error:
# sercmd sercmd 0.2 Copyright 2006 iptelorg GmbH This is free software with ABSOLUTELY NO WARRANTY. For details type `warranty'. sercmd> pkg.stats error: 500 - command pkg.stats not found sercmd>
Is it available only for 3.2.x and master branch? Because we are using 3.1.5. But take the PUA module from master branch for the fetch_row parameter you have patched ;-)
core.shmmem is ok.
Thanks a lot
Laura
On Wed, Dec 21, 2011 at 10:58 AM, Daniel-Constantin Mierla miconda@gmail.com wrote: > > Hello, > > you can see the available pkg via sercmd, sending command pkg.stats > (match > the entry for the pid printing the error). If there is no free memory, > then > might be a leak. > > You can attach with gdb to the pid printing these errors and walk to > pkg, > you see the commands for gdb at: > > http://www.asipto.com/pub/kamailio-devel-guide/#c04troubleshooting > > See if you have lot of allocated chunks from same place in source code > (ignore those at the beginning, mainly related to cfg parsing) and > send > the > details here. > > Cheers, > Daniel > > > On 12/21/11 10:44 AM, laura testi wrote: > > Hi, > we are using the PUA_XMPP and PUA modules from the master branch. When > the modules are started, everything are ok, the presence events from > XMPP are sent to kamailio SIP servers (PUBLISH/SUBSCRIBE) and cached > in the hash. But when there are several thousands records in the hash > tabel, the update_pua function called in the hashT_clean gives a lot > of "No memory left" error when the hashT_clean is waked up from the > time: > > ERROR: pua [send_subscribe.c]: No memory left > ERROR: pua [pua.c]: while building tm dlg_t structure > > The failed call is: > td = (dlg_t*)pkg_malloc(size); > if(td == NULL) > { > LM_ERR("No memory left\n"); > return NULL; > } > > in dlg_t* pua_build_dlg_t(ua_pres_t* presentity) function in > send_subscribe.c. The size is about 400 and something... It's > strange..... > > Is it the memory leak in the PUA module? > > > > I also try to increase the pkg_memory from 4MB default to 16MB, but it > doesn't help. > > > Any Idea? > > Thanks in advanced > > Laura > > _______________________________________________ > SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing > list > sr-users@lists.sip-router.org > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users > > > -- > Daniel-Constantin Mierla -- http://www.asipto.com > http://linkedin.com/in/miconda -- http://twitter.com/miconda
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list sr-users@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
-- Daniel-Constantin Mierla -- http://www.asipto.com http://linkedin.com/in/miconda -- http://twitter.com/miconda
-- Daniel-Constantin Mierla -- http://www.asipto.com http://linkedin.com/in/miconda -- http://twitter.com/miconda