From e55a5d32b7c1550ba39fea87ff8c6b0f8f47ffbb Mon Sep 17 00:00:00 2001 From: Alfonso Pinto Date: Sat, 3 Feb 2024 00:40:46 +0100 Subject: [PATCH 1/3] fix edge case that causes crash when unregistering (#247) --- .gitignore | 2 ++ src/mod/endpoints/mod_sofia/sofia.c | 16 ++++++++-------- src/mod/endpoints/mod_sofia/sofia_reg.c | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 0461c273de0..b54ed41c913 100644 --- a/.gitignore +++ b/.gitignore @@ -281,3 +281,5 @@ images/test_text.png src/mod/codecs/mod_amrwb/test/test_amrwb src/mod/endpoints/mod_sofia/test/sipp-based-tests + +DEBBUILD/ diff --git a/src/mod/endpoints/mod_sofia/sofia.c b/src/mod/endpoints/mod_sofia/sofia.c index 6eea2e22700..3adb82d6bf1 100644 --- a/src/mod/endpoints/mod_sofia/sofia.c +++ b/src/mod/endpoints/mod_sofia/sofia.c @@ -1515,14 +1515,6 @@ static void our_sofia_event_callback(nua_event_t event, if (sofia_private && sofia_private != &mod_sofia_globals.destroy_private && sofia_private != &mod_sofia_globals.keep_private) { if (!zstr(sofia_private->gateway_name)) { - if (event == nua_r_unregister && status != 401 && status != 407) { - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "Destroy handle after unregister for gateway %s.\n", sofia_private->gateway_name); - sofia_private_free(sofia_private); - nua_handle_bind(nh, NULL); - nua_handle_destroy(nh); - nh = NULL; - return; - } if (!(gateway = sofia_reg_find_gateway(sofia_private->gateway_name))) { return; } @@ -1633,6 +1625,14 @@ static void our_sofia_event_callback(nua_event_t event, } } + if (event == nua_r_unregister && status != 401 && status != 407 && status >= 200) { + if (gateway) { + switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "Mark gateway %s for destruction after unregister. Status %d.\n", sofia_private->gateway_name, status); + gateway->destroy = 1; + } + goto done; + } + if (sofia_test_pflag(profile, PFLAG_AUTH_ALL) && tech_pvt && tech_pvt->key && sip && (event < nua_r_set_params || event > nua_r_authenticate)) { sip_authorization_t const *authorization = NULL; diff --git a/src/mod/endpoints/mod_sofia/sofia_reg.c b/src/mod/endpoints/mod_sofia/sofia_reg.c index b693321c425..3ebe41c9bd6 100644 --- a/src/mod/endpoints/mod_sofia/sofia_reg.c +++ b/src/mod/endpoints/mod_sofia/sofia_reg.c @@ -461,6 +461,22 @@ void sofia_reg_check_gateway(sofia_profile_t *profile, time_t now) user_via = NULL; } + if(ostate == REG_STATE_DOWN && gateway_ptr->destroy) { + switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "Destroy sofia_private and nua_handler for gateway %s\n", gateway_ptr->name); + + if (gateway_ptr->sofia_private) { + sofia_private_free(gateway_ptr->sofia_private); + } + + if (gateway_ptr->nh) { + nua_handle_bind(gateway_ptr->nh, NULL); + nua_handle_destroy(gateway_ptr->nh); + gateway_ptr->nh = NULL; + } + + gateway_ptr->destroy = 0; + } + switch (ostate) { case REG_STATE_DOWN: case REG_STATE_NOREG: From d479848be0d438c1779f61ab982128edf443c5be Mon Sep 17 00:00:00 2001 From: Alfonso Pinto Date: Thu, 8 Feb 2024 15:34:02 +0100 Subject: [PATCH 2/3] Fix UNREGISTER flow (#253) --- src/mod/endpoints/mod_sofia/mod_sofia.c | 12 +++++++--- src/mod/endpoints/mod_sofia/mod_sofia.h | 1 + src/mod/endpoints/mod_sofia/sofia.c | 29 ++++++++++++++++++------- src/mod/endpoints/mod_sofia/sofia_reg.c | 24 +++++--------------- 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/mod/endpoints/mod_sofia/mod_sofia.c b/src/mod/endpoints/mod_sofia/mod_sofia.c index d1ffd7891a1..601c124ee38 100644 --- a/src/mod/endpoints/mod_sofia/mod_sofia.c +++ b/src/mod/endpoints/mod_sofia/mod_sofia.c @@ -3997,13 +3997,19 @@ static switch_status_t cmd_profile(char **argv, int argc, switch_stream_handle_t stream->write_function(stream, "+OK\n"); } else if ((gateway_ptr = sofia_reg_find_gateway(gname))) { if (gateway_ptr->state != REG_STATE_NOREG && gateway_ptr->state != REG_STATE_DOWN) { - gateway_ptr->retry = 0; - gateway_ptr->state = REG_STATE_UNREGISTER; + if (gateway_ptr->state == REG_STATE_FAILED && !sofia_test_flag(gateway_ptr, REG_FLAG_REGISTERED)) { + gateway_ptr->retry = 0; + gateway_ptr->state = REG_STATE_DOWN; + sofia_reg_fire_custom_gateway_state_event(gateway_ptr, 0, NULL); + } else { + gateway_ptr->retry = 0; + gateway_ptr->state = REG_STATE_UNREGISTER; + } stream->write_function(stream, "+OK\n"); - sofia_reg_release_gateway(gateway_ptr); } else { stream->write_function(stream, "-ERR NOREG gateway [%s] can't be unregistered!\n", gname); } + sofia_reg_release_gateway(gateway_ptr); } else { stream->write_function(stream, "Invalid gateway!\n"); } diff --git a/src/mod/endpoints/mod_sofia/mod_sofia.h b/src/mod/endpoints/mod_sofia/mod_sofia.h index 708770ce553..66d5d65d201 100644 --- a/src/mod/endpoints/mod_sofia/mod_sofia.h +++ b/src/mod/endpoints/mod_sofia/mod_sofia.h @@ -434,6 +434,7 @@ extern struct mod_sofia_globals mod_sofia_globals; typedef enum { REG_FLAG_AUTHED, REG_FLAG_CALLERID, + REG_FLAG_REGISTERED, /* No new flags below this line */ REG_FLAG_MAX diff --git a/src/mod/endpoints/mod_sofia/sofia.c b/src/mod/endpoints/mod_sofia/sofia.c index 3adb82d6bf1..c4954ce22ee 100644 --- a/src/mod/endpoints/mod_sofia/sofia.c +++ b/src/mod/endpoints/mod_sofia/sofia.c @@ -1625,14 +1625,6 @@ static void our_sofia_event_callback(nua_event_t event, } } - if (event == nua_r_unregister && status != 401 && status != 407 && status >= 200) { - if (gateway) { - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "Mark gateway %s for destruction after unregister. Status %d.\n", sofia_private->gateway_name, status); - gateway->destroy = 1; - } - goto done; - } - if (sofia_test_pflag(profile, PFLAG_AUTH_ALL) && tech_pvt && tech_pvt->key && sip && (event < nua_r_set_params || event > nua_r_authenticate)) { sip_authorization_t const *authorization = NULL; @@ -1679,6 +1671,27 @@ static void our_sofia_event_callback(nua_event_t event, case nua_r_info: break; case nua_r_unregister: + if (gateway && status != 401 && status != 407 && status >= 200) { + reg_state_t ostate = gateway->state; + + gateway->state = REG_STATE_DOWN; + gateway->status = SOFIA_GATEWAY_DOWN; + gateway->last_inactive = switch_epoch_time_now(NULL); + + if (gateway->sofia_private) { + sofia_private_free(gateway->sofia_private); + } + + if (gateway->nh) { + nua_handle_bind(gateway->nh, NULL); + nua_handle_destroy(gateway->nh); + gateway->nh = NULL; + } + if (ostate != gateway->state) { + sofia_reg_fire_custom_gateway_state_event(gateway, status, NULL); + } + } + break; case nua_r_unsubscribe: case nua_i_terminated: case nua_r_publish: diff --git a/src/mod/endpoints/mod_sofia/sofia_reg.c b/src/mod/endpoints/mod_sofia/sofia_reg.c index 3ebe41c9bd6..87f7fac5229 100644 --- a/src/mod/endpoints/mod_sofia/sofia_reg.c +++ b/src/mod/endpoints/mod_sofia/sofia_reg.c @@ -139,6 +139,8 @@ static void sofia_reg_kill_reg(sofia_gateway_t *gateway_ptr) if (gateway_ptr->state == REG_STATE_REGED || gateway_ptr->state == REG_STATE_UNREGISTER) { switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_NOTICE, "UN-Registering %s\n", gateway_ptr->name); nua_unregister(gateway_ptr->nh, NUTAG_URL(gateway_ptr->register_url), NUTAG_REGISTRAR(gateway_ptr->register_proxy), TAG_END()); + + // Returning here, nua_handle and sofia_private must be clean up in event *nua_r_unregister* with proper status return; } else { switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_NOTICE, "Destroying registration handle for %s\n", gateway_ptr->name); @@ -461,22 +463,6 @@ void sofia_reg_check_gateway(sofia_profile_t *profile, time_t now) user_via = NULL; } - if(ostate == REG_STATE_DOWN && gateway_ptr->destroy) { - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "Destroy sofia_private and nua_handler for gateway %s\n", gateway_ptr->name); - - if (gateway_ptr->sofia_private) { - sofia_private_free(gateway_ptr->sofia_private); - } - - if (gateway_ptr->nh) { - nua_handle_bind(gateway_ptr->nh, NULL); - nua_handle_destroy(gateway_ptr->nh); - gateway_ptr->nh = NULL; - } - - gateway_ptr->destroy = 0; - } - switch (ostate) { case REG_STATE_DOWN: case REG_STATE_NOREG: @@ -514,9 +500,6 @@ void sofia_reg_check_gateway(sofia_profile_t *profile, time_t now) case REG_STATE_UNREGISTER: sofia_reg_kill_reg(gateway_ptr); - gateway_ptr->state = REG_STATE_DOWN; - gateway_ptr->status = SOFIA_GATEWAY_DOWN; - gateway_ptr->last_inactive = switch_epoch_time_now(NULL); request++; // Only increment when FS sends out request break; case REG_STATE_UNREGED: @@ -2761,6 +2744,9 @@ void sofia_reg_handle_sip_r_register(int status, } } } + if (!sofia_test_flag(gateway, REG_FLAG_REGISTERED)) { + sofia_set_flag(gateway, REG_FLAG_REGISTERED); + } gateway->state = REG_STATE_REGISTER; break; case 100: From 79b1e1f50cd7e4e48f8e781f09a81ca0c1f8474a Mon Sep 17 00:00:00 2001 From: Alfonso Pinto Date: Tue, 13 Feb 2024 13:22:35 +0100 Subject: [PATCH 3/3] TEL-5929: Improve race condition between REGISTER/INVITE (#255) Set GW as SOFIA_GATEWAY_UP as soon as 200 OK is received for REGISTER Add new REG CPS for shutdown --- src/mod/endpoints/mod_sofia/mod_sofia.h | 1 + src/mod/endpoints/mod_sofia/sofia.c | 6 ++++-- src/mod/endpoints/mod_sofia/sofia_reg.c | 4 ++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mod/endpoints/mod_sofia/mod_sofia.h b/src/mod/endpoints/mod_sofia/mod_sofia.h index 66d5d65d201..1bf5b82bda4 100644 --- a/src/mod/endpoints/mod_sofia/mod_sofia.h +++ b/src/mod/endpoints/mod_sofia/mod_sofia.h @@ -733,6 +733,7 @@ struct sofia_profile { sofia_gateway_t *gateways; sofia_gateway_t *next_check_gateway_ptr; unsigned int gateway_reg_max_cps; + unsigned int gateway_shutdown_reg_max_cps; unsigned int gateway_unreg_max_yield_ms; //su_home_t *home; switch_hash_t *chat_hash; diff --git a/src/mod/endpoints/mod_sofia/sofia.c b/src/mod/endpoints/mod_sofia/sofia.c index c4954ce22ee..102debe37ad 100644 --- a/src/mod/endpoints/mod_sofia/sofia.c +++ b/src/mod/endpoints/mod_sofia/sofia.c @@ -3672,7 +3672,7 @@ void *SWITCH_THREAD_FUNC sofia_profile_thread_run(switch_thread_t *thread, void /* Mark all gateways as deleted and set REG_STATE_UNREGISTER state on REG gateways */ sofia_glue_del_every_gateway(profile); /* This prevent doing batch request for reg check */ - profile->gateway_reg_max_cps = 0; + profile->gateway_reg_max_cps = profile->gateway_shutdown_reg_max_cps; /* First call will unregister and set state to DOWN so a gateway is ready for deletion */ sofia_reg_check_gateway(profile, switch_epoch_time_now(NULL)); sofia_sub_check_gateway(profile, switch_epoch_time_now(NULL)); @@ -3981,7 +3981,7 @@ static void parse_gateways(sofia_profile_t *profile, switch_xml_t gateways_tag, switch_mutex_lock(mod_sofia_globals.hash_mutex); if ((gp = switch_core_hash_find(mod_sofia_globals.gateway_hash, name)) && (gp = switch_core_hash_find(mod_sofia_globals.gateway_hash, pkey)) && !gp->deleted) { - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_WARNING, "Ignoring duplicate gateway '%s'\n", name); + //switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_WARNING, "Ignoring duplicate gateway '%s'\n", name); switch_mutex_unlock(mod_sofia_globals.hash_mutex); free(pkey); goto skip; @@ -5860,6 +5860,8 @@ switch_status_t config_sofia(sofia_config_t reload, char *profile_name) profile->max_auth_validity = atoi(val); } else if (!strcasecmp(var, "gateway-reg-max-cps")) { profile->gateway_reg_max_cps = atoi(val); + } else if (!strcasecmp(var, "gateway-shutdown-reg-max-cps")) { + profile->gateway_shutdown_reg_max_cps = atoi(val); } else if (!strcasecmp(var, "auth-require-user")) { if (switch_true(val)) { sofia_set_pflag(profile, PFLAG_AUTH_REQUIRE_USER); diff --git a/src/mod/endpoints/mod_sofia/sofia_reg.c b/src/mod/endpoints/mod_sofia/sofia_reg.c index 87f7fac5229..4afa70e7931 100644 --- a/src/mod/endpoints/mod_sofia/sofia_reg.c +++ b/src/mod/endpoints/mod_sofia/sofia_reg.c @@ -2747,6 +2747,10 @@ void sofia_reg_handle_sip_r_register(int status, if (!sofia_test_flag(gateway, REG_FLAG_REGISTERED)) { sofia_set_flag(gateway, REG_FLAG_REGISTERED); } + if (gateway->status != SOFIA_GATEWAY_UP) { + gateway->status = SOFIA_GATEWAY_UP; + gateway->uptime = switch_time_now(); + } gateway->state = REG_STATE_REGISTER; break; case 100: