git:4.2:edbed4cc: dmq: be sure dmq_init_callback_done is initialized before accessing it
Module: kamailio Branch: 4.2 Commit: edbed4cc15a250e4b7c12d3d322b8a6575582c21 URL: https://github.com/kamailio/kamailio/commit/edbed4cc15a250e4b7c12d3d322b8a65...
Author: Daniel-Constantin Mierla miconda@gmail.com Committer: Daniel-Constantin Mierla miconda@gmail.com Date: 2015-01-08T16:02:31+01:00
dmq: be sure dmq_init_callback_done is initialized before accessing it
(cherry picked from commit b29a45f0a23ef0f1a26598a45d3e6eddc9dbedb5)
---
Modified: modules/dmq/notification_peer.c
---
Diff: https://github.com/kamailio/kamailio/commit/edbed4cc15a250e4b7c12d3d322b8a65... Patch: https://github.com/kamailio/kamailio/commit/edbed4cc15a250e4b7c12d3d322b8a65...
---
diff --git a/modules/dmq/notification_peer.c b/modules/dmq/notification_peer.c index b493717..2e87c77 100644 --- a/modules/dmq/notification_peer.c +++ b/modules/dmq/notification_peer.c @@ -29,7 +29,7 @@ str notification_content_type = str_init("text/plain"); dmq_resp_cback_t notification_callback = {¬ification_resp_callback_f, 0};
-int *dmq_init_callback_done; +int *dmq_init_callback_done = 0;
/** @@ -229,7 +229,7 @@ int dmq_notification_callback(struct sip_msg* msg, peer_reponse_t* resp, dmq_nod ¬ification_callback, maxforwards, ¬ification_content_type); } pkg_free(response_body); - if (!*dmq_init_callback_done) { + if (dmq_init_callback_done && !*dmq_init_callback_done) { *dmq_init_callback_done = 1; run_init_callbacks(); } @@ -325,7 +325,7 @@ int notification_resp_callback_f(struct sip_msg* msg, int code, if(code == 200) { nodes_recv = extract_node_list(node_list, msg); LM_DBG("received %d new or changed nodes\n", nodes_recv); - if (!*dmq_init_callback_done) { + if (dmq_init_callback_done && !*dmq_init_callback_done) { *dmq_init_callback_done = 1; run_init_callbacks(); }
Hi,
This issue comes form the fact that kamailio is already handling SIP messages before the modules are initialized. I found some other occurrences also.
It would be more efficient if the "normal" runtime code wouldn't have to check all the pointers which should have been initialized by the module init code.
What would it take to guarantee all modules have run their init code before handling SIP messages?
Hello,
kamailio is calling the module destroy function without the module being fully initialized, so it is good at least to initialize to a value at declaration and check in the destroy function.
Then, ensuring that no traffic is processed could be hard if some modules do such things on their own.
Kamailio is starting listening once starting forking children (main process doesn't listen on 'sip' sockets). If I understood right, the (last) crashes reported by Olle to me (via irc) happened after proper startup - a sip request was sent and a reply was received. At least a backtrace I received was:
#0 0x00007fff565b2f24 in ?? () No symbol table info available. #1 0x000000010eaa2b76 in notification_resp_callback_f (msg=0x109d47080, code=200, node=0x10acdc5d8, param=0x0) at notification_peer.c:330 #2 0x000000010ea8a829 in dmq_tm_callback (t=0x10acdcad8, type=1024, ps=0x7fff565b5e88) at dmq_funcs.c:64 cb_param = (dmq_cback_param_t *) 0x10acdc588 #3 0x000000010a82a6d8 in run_trans_callbacks_internal (cb_lst=0x10acdcb48, type=1024, trans=0x10acdcad8, params=0x7fff565b5e88) at t_hooks.c:268 #4 0x000000010a82a82d in run_trans_callbacks (type=1024, trans=0x10acdcad8, req=0x0, rpl=0x109d47080, code=200) at t_hooks.c:295 #5 0x000000010a883b84 in local_reply (t=0x10acdcad8, p_msg=0x109d47080, branch=0, msg_status=200, cancel_data=0x7fff565b6748) at t_reply.c:2039 #6 0x000000010a886cbf in reply_received (p_msg=0x109d47080) at t_reply.c:2395 #7 0x0000000109712ca3 in do_forward_reply (msg=0x109d47080, mode=0) at forward.c:747 #8 0x000000010971254a in forward_reply (msg=0x109d47080) at forward.c:849 #9 0x00000001097ebfa1 in receive_msg (buf=0x109c615e0 "SIP/2.0 200 OK ...", ...) at receive.c:255 #10 0x00000001099a44d7 in udp_rcv_loop () at udp_server.c:496
On the other hand, some where from init time -- it might have used the event route for htable mod init, adding items in hash table there. So the htable module has to handle this case or dmq be prepared.
#0 0x00007fff5a18ef04 in ?? () No symbol table info available. #1 0x000000010aec9800 in dmq_notification_callback (msg=0x1071023f0, resp=0x7fff5a193780, dmq_node=0x107100338) at notification_peer.c:236 #2 0x000000010aed0271 in worker_loop (id=0) at worker.c:101 #3 0x000000010aea9cba in child_init (rank=0) at dmq.c:293 #4 0x0000000105cfba35 in init_mod_child (m=0x10614f400, rank=0) at sr_module.c:898
More details on what I discovered while troubleshooting is that the init callback in dmq is executed also via the callbacks for message received, as part of the last peer in the list which seems to be kind of special structure content.
Hopefully the above can help you get some leads of what goes wrong (note that some lines might have small offset, as I added some checks at some point and didn't correlate the time of commits with the backtraces).
Cheers, Daniel
On 09/01/15 11:01, Alex Hermann wrote:
Hi,
This issue comes form the fact that kamailio is already handling SIP messages before the modules are initialized. I found some other occurrences also.
It would be more efficient if the "normal" runtime code wouldn't have to check all the pointers which should have been initialized by the module init code.
What would it take to guarantee all modules have run their init code before handling SIP messages?
-
Alex Hermann -
Daniel-Constantin Mierla