From 6763dda90fb8faa0ac03a65f23dcf4be54a246ef Mon Sep 17 00:00:00 2001 From: Thomas Guebels Date: Mon, 28 Oct 2024 17:29:48 +0100 Subject: [PATCH] pjsip_transport_events: Avoid monitor destruction When a transport is disconnected, several events can arrive following each other. The first event will be PJSIP_TP_STATE_DISCONNECT and it will trigger the destruction of the transport monitor object. The lookup for the transport monitor to destroy is done using the transport key, that contains the transport destination host:port. A reconnect attempt by pjsip will be triggered as soon something needs to send a packet using that transport. This can happen directly after a disconnect since ca Subsequent events can arrive later like PJSIP_TP_STATE_DESTROY and will also try to trigger the destruction of the transport monitor if not already done. Since the lookup for the transport monitor to destroy is done using the transport key, it can match newly created transports towards the same destination and destroy their monitor object. Because of this, it was sometimes not possible to monitor a transport after one or more disconnections. This fix adds an additional check on the transport pointer to ensure only a monitor for that specific transport is removed. Fixes: #923 --- res/res_pjsip/pjsip_transport_events.c | 35 ++++++++++++++++---------- res/res_pjsip_outbound_registration.c | 7 ++++-- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/res/res_pjsip/pjsip_transport_events.c b/res/res_pjsip/pjsip_transport_events.c index c6c5d21af3..aa575b7d58 100644 --- a/res/res_pjsip/pjsip_transport_events.c +++ b/res/res_pjsip/pjsip_transport_events.c @@ -138,27 +138,36 @@ static void transport_monitor_dtor(void *vdoomed) */ static void transport_state_do_reg_callbacks(struct ao2_container *transports, pjsip_transport *transport) { + struct ao2_iterator *monitor_iter; struct transport_monitor *monitored; char key[IP6ADDR_COLON_PORT_BUFLEN]; + int idx; AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(transport, key); - monitored = ao2_find(transports, key, OBJ_SEARCH_KEY | OBJ_UNLINK); - if (monitored) { - int idx; - - for (idx = AST_VECTOR_SIZE(&monitored->monitors); idx--;) { - struct transport_monitor_notifier *notifier; - - notifier = AST_VECTOR_GET_ADDR(&monitored->monitors, idx); - ast_debug(3, "Transport %s(%s,%s) RefCnt: %ld : running callback %p(%p)\n", - monitored->key, monitored->transport->obj_name, - monitored->transport->type_name, - pj_atomic_get(monitored->transport->ref_cnt), notifier->cb, notifier->data); - notifier->cb(notifier->data); + monitor_iter = ao2_find(transports, key, OBJ_SEARCH_KEY | OBJ_MULTIPLE); + while ((monitored = ao2_iterator_next(monitor_iter))) { + if (monitored->transport == transport) { + ao2_unlink(transports, monitored); + for (idx = AST_VECTOR_SIZE(&monitored->monitors); idx--;) { + struct transport_monitor_notifier *notifier; + + notifier = AST_VECTOR_GET_ADDR(&monitored->monitors, idx); + ast_debug(3, "Transport %s(%s,%s) RefCnt: %ld : running callback %p(%p)\n", + monitored->key, monitored->transport->obj_name, + monitored->transport->type_name, + pj_atomic_get(monitored->transport->ref_cnt), notifier->cb, notifier->data); + notifier->cb(notifier->data); + } + } else { + ast_debug(3, "Transport %s(%s,%s) RefCnt: %ld : ignored not matching %s\n", + monitored->key, monitored->transport->obj_name, + monitored->transport->type_name, + pj_atomic_get(monitored->transport->ref_cnt), transport->obj_name); } ao2_ref(monitored, -1); } + ao2_iterator_destroy(monitor_iter); } static void verify_log_result(int log_level, const pjsip_transport *transport, diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c index 59a9fde5a5..b81057f083 100644 --- a/res/res_pjsip_outbound_registration.c +++ b/res/res_pjsip_outbound_registration.c @@ -1145,6 +1145,7 @@ static int monitor_matcher(void *a, void *b) static void registration_transport_monitor_setup(const char *transport_key, const char *registration_name) { char *monitor; + enum ast_transport_monitor_reg monitor_res; monitor = ao2_alloc_options(strlen(registration_name) + 1, NULL, AO2_ALLOC_OPT_LOCK_NOLOCK); @@ -1158,8 +1159,10 @@ static void registration_transport_monitor_setup(const char *transport_key, cons * register the monitor. We might get into a message spamming infinite * loop of registration, shutdown, reregistration... */ - ast_sip_transport_monitor_register_replace_key(transport_key, registration_transport_shutdown_cb, - monitor, monitor_matcher); + if((monitor_res = ast_sip_transport_monitor_register_replace_key(transport_key, registration_transport_shutdown_cb, + monitor, monitor_matcher))) { + ast_log(LOG_NOTICE, "Failed to register transport monitor for regisration %s: %d\n", registration_name, monitor_res); + } ao2_ref(monitor, -1); }