[sr-dev] [SR-Users] No memory left in send_subscribe (PUA) from update_pua while building the tm dlg_t structure

Daniel-Constantin Mierla miconda at gmail.com
Thu Dec 29 16:41:52 CET 2011


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=b93149c756d3e983c70608938f1142ed43ee1834

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 at 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 at 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 at gmail.com>  wrote:
>>
>> Hello,
>>
>> can you try with this patch:
>>
>> http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=1b3cfa60a5b5c7d435704d44b7c495b7e6aa84c8
>>
>> 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 at 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 at 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 at 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 at 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 at 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 at 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20111229/13dafcad/attachment-0001.htm>


More information about the sr-dev mailing list