[sr-dev] git:richard.good/diameter_rx_media: modules/ims_qos: Fixed race condition between dialog teardown and CDP teardown

Module: sip-router Branch: richard.good/diameter_rx_media Commit: 5534a714b8c91ee5b6ae3168282bb17c9bf7511e URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5534a71… Author: Richard Good &lt;richard.good(a)smilecoms.com&gt; Committer: Richard Good &lt;richard.good(a)smilecoms.com&gt; Date: Mon Jul 8 16:15:55 2013 +0200 modules/ims_qos: Fixed race condition between dialog teardown and CDP teardown - rx_authdata.c/h: added new parm to rx_authsessiondata: must_terminate_dialog - cdpeventprocessor.c: Use new parm to ensure that when CDP session is terminated due to transport plane event (e.g. Diameter ASR) the dialog is terminated - If the CDP session is terminated due to signalling plane event (e.g. SIP BYE) the dialog is not terminated again --- modules/ims_qos/cdpeventprocessor.c | 73 +++++++++++++++------------------- modules/ims_qos/mod.c | 2 +- modules/ims_qos/rx_authdata.c | 2 + modules/ims_qos/rx_authdata.h | 1 + 4 files changed, 36 insertions(+), 42 deletions(-) diff --git a/modules/ims_qos/cdpeventprocessor.c b/modules/ims_qos/cdpeventprocessor.c index c3ea661..1017c5c 100644 --- a/modules/ims_qos/cdpeventprocessor.c +++ b/modules/ims_qos/cdpeventprocessor.c @@ -200,47 +200,38 @@ void cdp_cb_event_process() { switch (ev->event) { case AUTH_EV_SESSION_TIMEOUT: case AUTH_EV_SESSION_GRACE_TIMEOUT: - case AUTH_EV_SESSION_LIFETIME_TIMEOUT: - LM_DBG("Rx CDP Session: AUTH EV SESSION TIMEOUT or GRACE TIMEOUT or LIFE TIMEOUT\n"); + case AUTH_EV_RECV_ASR: + LM_DBG("Received notification of ASR from transport plane or CDP timeout for CDP session with Rx session ID: [%.*s] and associated contact [%.*s]" + " and domain [%.*s]\n", + rx_session_id->len, rx_session_id->s, + p_session_data->registration_aor.len, p_session_data->registration_aor.s, + p_session_data->domain.len, p_session_data->domain.s); + if (p_session_data->subscribed_to_signaling_path_status) { - LM_DBG("Received notification of CDP timeout of CDP session with Rx session ID: [%.*s] and associated contact [%.*s]" - " and domain [%.*s]\n", - rx_session_id->len, rx_session_id->s, - p_session_data->registration_aor.len, p_session_data->registration_aor.s, - p_session_data->domain.len, p_session_data->domain.s); LM_DBG("This is a subscription to signalling bearer session"); + ; + //nothing to do here - just wait for AUTH_EV_SERVICE_TERMINATED event } else { - LM_DBG("Received notification of CDP timeout of CDP session with Rx session ID: [%.*s] and associated contact [%.*s]" - " and domain [%.*s]\n", - rx_session_id->len, rx_session_id->s, - p_session_data->registration_aor.len, p_session_data->registration_aor.s, - p_session_data->domain.len, p_session_data->domain.s); LM_DBG("This is a media bearer session session"); - LM_DBG("Terminating dialog with callid, ftag, ttag: [%.*s], [%.*s], [%.*s]\n", - p_session_data->callid.len, p_session_data->callid.s, - p_session_data->ftag.len, p_session_data->ftag.s, - p_session_data->ttag.len, p_session_data->ttag.s); - dlgb.terminate_dlg(&p_session_data->callid, - &p_session_data->ftag, &p_session_data->ttag, NULL, - &release_reason); + //this is a media bearer session that was terminated from the transport plane - we need to terminate the associated dialog + //so we set p_session_data->must_terminate_dialog to 1 and when we receive AUTH_EV_SERVICE_TERMINATED event we will terminate the dialog + p_session_data->must_terminate_dialog = 1; } break; case AUTH_EV_SERVICE_TERMINATED: - LM_DBG("Rx CDP Session: Service terminated\n"); - + LM_DBG("Received notification of CDP TERMINATE of CDP session with Rx session ID: [%.*s] and associated contact [%.*s]" + " and domain [%.*s]\n", + rx_session_id->len, rx_session_id->s, + p_session_data->registration_aor.len, p_session_data->registration_aor.s, + p_session_data->domain.len, p_session_data->domain.s); + if (p_session_data->subscribed_to_signaling_path_status) { - LM_DBG("Received notification of CDP TERMINATE of CDP session with Rx session ID: [%.*s] and associated contact [%.*s]" - " and domain [%.*s]\n", - rx_session_id->len, rx_session_id->s, - p_session_data->registration_aor.len, p_session_data->registration_aor.s, - p_session_data->domain.len, p_session_data->domain.s); LM_DBG("This is a subscription to signalling bearer session"); - //instead of removing the contact from usrloc_pcscf we just change the state of the contact to TERMINATE_PENDING_NOTIFY //pcscf_registrar sees this, sends a SIP PUBLISH and on SIP NOTIFY the contact is deleted - + if (ul.register_udomain(p_session_data->domain.s, &domain) < 0) { LM_DBG("Unable to register usrloc domain....aborting\n"); @@ -258,21 +249,21 @@ void cdp_cb_event_process() { } ul.unlock_udomain(domain, &p_session_data->registration_aor); } else { - LM_DBG("Received notification of CDP TERMINATE of CDP session with Rx session ID: [%.*s] and associated contact [%.*s]" - " and domain [%.*s]\n", - rx_session_id->len, rx_session_id->s, - p_session_data->registration_aor.len, p_session_data->registration_aor.s, - p_session_data->domain.len, p_session_data->domain.s); LM_DBG("This is a media bearer session session"); - LM_DBG("Terminating dialog with callid, ftag, ttag: [%.*s], [%.*s], [%.*s]\n", - p_session_data->callid.len, p_session_data->callid.s, - p_session_data->ftag.len, p_session_data->ftag.s, - p_session_data->ttag.len, p_session_data->ttag.s); - dlgb.terminate_dlg(&p_session_data->callid, - &p_session_data->ftag, &p_session_data->ttag, NULL, - &release_reason); + + //we only terminate the dialog if this was triggered from the transport plane or timeout - i.e. if must_terminate_dialog is set + //if this was triggered from the signalling plane (i.e. someone hanging up) then we don'y need to terminate the dialog + if (p_session_data->must_terminate_dialog) { + LM_DBG("Terminating dialog with callid, ftag, ttag: [%.*s], [%.*s], [%.*s]\n", + p_session_data->callid.len, p_session_data->callid.s, + p_session_data->ftag.len, p_session_data->ftag.s, + p_session_data->ttag.len, p_session_data->ttag.s); + dlgb.terminate_dlg(&p_session_data->callid, + &p_session_data->ftag, &p_session_data->ttag, NULL, + &release_reason); + } } - + //free callback data if (p_session_data) { shm_free(p_session_data); diff --git a/modules/ims_qos/mod.c b/modules/ims_qos/mod.c index 20b16dc..1ed636d 100644 --- a/modules/ims_qos/mod.c +++ b/modules/ims_qos/mod.c @@ -293,7 +293,7 @@ void callback_for_cdp_session(int event, void *session) { //only put the events we care about on the event stack if (event == AUTH_EV_SESSION_TIMEOUT || event == AUTH_EV_SESSION_GRACE_TIMEOUT || - event == AUTH_EV_SESSION_LIFETIME_TIMEOUT || + event == AUTH_EV_RECV_ASR || event == AUTH_EV_SERVICE_TERMINATED) { LOG(L_DBG, "callback_for_cdp session(): called with event %d and session id [%.*s]\n", event, rx_session_id->len, rx_session_id->s); diff --git a/modules/ims_qos/rx_authdata.c b/modules/ims_qos/rx_authdata.c index d75d88c..e828634 100644 --- a/modules/ims_qos/rx_authdata.c +++ b/modules/ims_qos/rx_authdata.c @@ -79,6 +79,7 @@ int create_new_regsessiondata(str* domain, str* aor, rx_authsessiondata_t** sess memset(p_session_data, 0, len); p_session_data->subscribed_to_signaling_path_status = 1; + p_session_data->must_terminate_dialog = 0; /*irrelevent for reg session data this will always be 0 */ char* p = (char*)(p_session_data + 1); p_session_data->domain.s = p; @@ -112,6 +113,7 @@ int create_new_callsessiondata(str* callid, str* ftag, str* ttag, rx_authsession } memset(call_session_data, 0, len); call_session_data->subscribed_to_signaling_path_status = 0; //this is for a media session not regitration + call_session_data->must_terminate_dialog = 0; //this is used to determine if the dialog must be torn down when the CDP session terminates char *p = (char*)(call_session_data + 1); diff --git a/modules/ims_qos/rx_authdata.h b/modules/ims_qos/rx_authdata.h index 69373c0..4310c13 100644 --- a/modules/ims_qos/rx_authdata.h +++ b/modules/ims_qos/rx_authdata.h @@ -69,6 +69,7 @@ typedef struct rx_authsessiondata { int subscribed_to_signaling_path_status; // 0 not subscribed 1 is subscribed str domain; //the domain the registration aor belongs to (for registration) str registration_aor; //the aor if this rx session is a subscription to signalling status + int must_terminate_dialog; //0 means when this session terminates it must not terminate the relevant dialog, 1 means it must terminate the dialog } rx_authsessiondata_t; int create_new_regsessiondata(str* domain, str* aor, rx_authsessiondata_t** session_data);