Hello Laura,
I applied most of the patches, apart the second one for hash.c, related
to checking for temporary dialog. I need to look a bit more at it, since
there was some work in this regard:
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=b93…
If I understood right, it is about NOTIFY getting processed faster than
200OK for SUBSCRIBE. Do I remember correctly, you are running 3.1.x? It
might not be in branch 3.1.
Cheers,
Daniel
On 12/28/11 6:15 PM, laura testi wrote:
> Hi Daniel,
> Yes, all patches are related to the PUA module. Please find the
> attachments for both diff files and new files with applied patches If
> you thinks they are useful. Now the old files should be the latest
> version of the master branch ;-) Unfortunately I can not use git here.
> We have tested the patches, now no memory leak any more; and no more
> error for NOTIFY (dialog not found)...Please note, our use case is the
> pua_xmpp, now it works fine. I don't know if rls has the same problem.
> Thank you veru much for your helps again.
>
> Some explanations:
>
> In the send_subscribe.c,
> - I just comment out the line 1172 so that the pua_free_tm_dlg(td)
> do the right job later as you done also in this case.
>
> In the hash.c,
> - in the get_dialog function, I add the check of "p->to_tag.len> 0"
> in the string compare conditions;
>
> - in the is_dialog function, instead of check only the real dialog,
> the check of also the temporary dialog is added, this also helps to
> avoid the error of no dialog found for the NOTIFY if the NOTIFY is
> received before the 202/200 OK of the SUBSCRIBE.
>
> in the pua.c,
> - a few changes in the update_pua function:
> - apply the same kind of free td and td->route_set, unfortunately
> I can not re-use pua_free_tm_dlg which is local for send_subscribe.c.
> - change from goto error to goto done
>
>
> Best Regards,
> Laura
>
>
>
> On Wed, Dec 28, 2011 at 2:39 PM, Daniel-Constantin Mierla
> <miconda(a)gmail.com> wrote:
>> Hello,
>>
>>
>> On 12/27/11 1:51 PM, laura testi wrote:
>>
>> 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
>>
>>
>> ok, thanks, I will look over. But how did you do the patches, since it does
>> not take my latest patch, seems to be against an older version, because the
>> indentation is not there?
>>
>> I cannot apply it like this, maybe you can tell the files and lines you
>> changed, otherwise is hard to track.
>>
>> Are both patches to the pua module? You are doing the patch with files only,
>> it is more convenient to call the diff with path to the module, in this way
>> is easy to spot in which module to apply the path -- i.e., use diff from
>> root folder of kamailio, like:
>>
>> diff -u modules/abc/oldfile modules/abc/newfile
>>
>> Since you are sending a lot of patches, maybe it will work better if you
>> just clone the git, make the patch against the master branch -- change the
>> file in the master branch and then just do:
>>
>> git diff> path/to/save/patch.file
>>
>> Some info that could be useful for working with git and patches, including
>> backporting, at:
>>
>> http://www.kamailio.org/wiki/devel/backporting-to-3.2.x
>>
>> Cheers,
>> Daniel
>>
>>
>>
>>
>> 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(a)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(a)gmail.com> wrote:
>>
>> Hello,
>>
>> can you try with this patch:
>>
>> http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=1b3cfa6…
>>
>> 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(a)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(a)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(a)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(a)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
>>
>>
>>
>> _______________________________________________
>> SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
>> sr-users(a)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(a)lists.sip-router.org
>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
--
Daniel-Constantin Mierla -- http://www.asipto.comhttp://linkedin.com/in/miconda -- http://twitter.com/miconda