Kamailio-5.4.0 : Crashes from keepalive module #2448
Labels
Comments
|
Do you use Because looking at the code, when using the function in config, is not setting any state change callback function, which seems to be the one missing at the top of the backtrace (by looking what is at keepalive_core.c:126). |
|
I have used ka_add_destination in config file. |
|
Hi, @miconda |
|
I am not the developer of the module, I just looked quickly in the code and it seemed that it should not execute the callback that caused the crash if the function was used in the config. That's why I asked if you use it inside a custom module. If I get some spare time, I will look again, but I cannot say if it is going to be fixed before the next minor release in 5.4 -- this is expected to happen soon, maximum 1-2 weeks and I expect to be quite busy getting Kamailio World online event organized. The original developer changed the job and is no longer active, iirc, but there were other devs contributing to this module lately, maybe they can have a look as well -- pinging @luismartingil for it. |
|
@miconda , Thanks i understand. |
|
Supposing you still have the core file, can you get the output of next commands executed inside gdb: |
|
Hi,
```
(gdb) frame 1
#1 0x00007ff5bbb2ddd8 in run_trans_callbacks_internal
(cb_lst=0x7ff47c9f86a8, type=1024, trans=0x7ff47c9f8630,
params=0x7ffdb5b643e0)
at t_hooks.c:258
258 cbp->callback( trans, type, params );
(gdb) p *ka_dest
No symbol "ka_dest" in current context.
(gdb)
```
|
|
In the first backtrace, the frame 1 was different: Did you get another core file? I need the output of |
|
@NGSegovia FYI |
|
I have generated fresh core dump : |
|
Can you make a minimal kamailio config to reproduce this issue? The uri field is compromised, maybe due to a free/destroy of the keepalive structure. Are you adding/removing ka destinations on the fly/during runtime? |
|
I'd say the reporter is adding the destination from another module, and not from .cfg. |
|
@NGSegovia - it is what I asked in my first comments, based on what I saw in the code. But the last backtrace shows also URI field being invalid, so maybe the ka structure is compromised/overwritten -- like already freed/destroyed while the keepalive was ongoing, and another process got the memory zone and wrote there. That's why I asked to see if the destination is eventually removed from the KA list. I am not the developer of the module, but does not look like there is a protection for this case (provided that destinations can be removed at any time)... |
|
In principle, there is a lock used for destination list: https://github.com/kamailio/kamailio/blob/master/src/modules/keepalive/keepalive_api.c#L178 mmm, I wonder why URI seems corrupted, but didn't get to corrupt "owner" in the backtrace. |
|
The timer for keepalive is stopped, but here it seems to come a pointer which was stored inside sip transaction structure and used when the transaction completes (on 408 timeout or 200ok response). If between the options and final response the ka destination structure is destroyed, then the tm callback will provide a pointer to an address that can be used for something else at that moment. In such case it is better to store in transaction a unique id for ka destination structure and on tm transaction callback to search and see if it is found, locking the structure while is updated. |
|
@miconda , Yes i am adding and removing on the fly. I am adding(using ka_add_destination) to KA list on successful registration and removing(using ka_del_destination) when registration expires. @NGSegovia , I am adding destinations from CFG file. |
|
@NGSegovia - not related to the issue itself, but is there any specific reason to do this complex logic for keepalives? If it is for registrations, the usrloc module in kamailio v5.4.x can do keepalive (storing also the round trip time). Otherwise, the issue seems to be related to keeping the pointer to keepalive destinations structure in the transaction and can be destroyed until transaction completes. If no other devel looks at it, I will try to get familiar with the code of the module and come up with a fix after Kamailio World Online event. |
|
I preferred this module over usrloc module because i dont store registrations on kamailio. It is stored on application server behind Kamailio. So it was easy for me to use keepalive module to send KA instead of usrloc. |
|
@miconda the problem you pointed is the same pointed in this message, right? https://github.com/kamailio/kamailio/blob/master/src/modules/keepalive/keepalive_core.c#L102 BTW I did some code, hopefully fixing this, but I didn't cleaned-up and test properly yet, but you can take a look. https://github.com/NGSegovia/kamailio/tree/keepalive_fix_race_condition_tm_request |
|
@NGSegovia , The changes you made has fixed the issue. Kamailio service is running since last 2 hours and I am not able to reproduce issue. @miconda , I understand you are busy with Kamailio world, Is there any chances for this fix to be available in next stable release ? |
|
@NGSegovia - yes, that kind of problem. The approach in the fix is correct, just wondering if worth adding dependency on external libuuid library instead of using the internal core/utils/sruid.{c,h}, like it is done for other modules. I planned to extend the internal sruid code to be able to use libuuid via the By using the internal sruid backporting will be smooth, because there is no extra dependency, so it does not break packaging (debs, rpms, ... stay the same). |
10 tasks
|
Not a problem @miconda , updated. |
|
@NGSegovia , I tested latest changes under Now it is crashing again but from different place : |
|
@sagarmalam I have updated the PR, please check again. Thanks! |
|
@NGSegovia , It is working fine now. Thanks a lot. Question : |
|
No idea @sagarmalam |
|
ok. Thanks. I will share my results. |
|
Hello Again, I was able to reproduce again. It crashed from the same point and it seems that frequency of this issue has reduced but it is not fixed yet. OPTIONS : REGISTER : Kamailio has dispatched 200 OK for register at 06:33:58.187520 and it received 200 OK for OPTIONS at 06:33:58.411501. Kamailio crashed exactly at a same time. Core Dump : Thanks |
|
Hello |
|
@NGSegovia , While testing latest fixes we have found that Keepalive module is leaking memory. We are testing with 7K destinations with time interval of 20 seconds At Wed Sep 9 09:02:07 GMT 2020 it is 3 MB Kamailio core dumped after running for 24 hours with 5GB SHM memory. Kamailio logs : Please get back to me in case you need any more information. |
sagarmalam
referenced
this issue
in NGSegovia/kamailio
Sep 9, 2020
… struct to tm - This avoids a race condition that may happen processing tm_request callbacl - Allows to identify uniquely a ka_dest record
10 tasks
|
@sagarmalam merged PR fixing this problem. Re-open if there are still issues. |
|
@NGSegovia , Thanks for all the efforts in fix this issue. I think i have found regression issue. KA timeout parameter is not working anymore with latest changes. It keeps on sending KA even after failing for number of times defined in ka_timeout mod param. |
|
Hi @sagarmalam , do you mean BTW id you set that parameter to "0" means no limit. Not sure if that is the case. Anyway, I can't find the relation with changes done here and this parameter. Can you try again a open a new issue with more information if you still have problems? Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
I am using keepalive module to send keepalives to all the destinations registered with an application server which sits behind kamailio.
When Kamailio recieves 200 Ok. I take URI contact from 200 OK and pass it as a param to ka_add_destination function.Most of the time it works fine but sometimes kamailio crashes when it recieves 200 OK of OPTIONS.
Troubleshooting
Reproduction
I am able to reproduce it easily in below scenario :
All the URIs which we are feeding to KA module has this syntax :
sip:2003@10.50.8.11:9090;alias=10.50.8.1~5060~1;pb-ip=35.185.177.49;pb-pt=5060;tp=udp- We are calling handle_ruri_alias() function which will set
$du =sip:10.50.8.1:5060;transport=udp- Modify RURI from
sip:2003@10.50.8.11:9090;alias=10.50.8.1~5060~1;pb-ip=35.185.177.49;pb-pt=5060;tp=udptosip:2003@35.185.177.49:5060;pb-ip=35.185.177.49;pb-pt=5060;transport=udp- and relay to 10.50.8.1
10.50.8.1is another instance of Kamailio which will relay OPTION packet to final destinationDebugging Data
Log Messages
SIP Traffic
Possible Solutions
Additional Information
kamailio -vThe text was updated successfully, but these errors were encountered: