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
(cherry picked from commit 6763dda90fb8faa0ac03a65f23dcf4be54a246ef)

diff --git a/res/res_pjsip/pjsip_transport_events.c b/res/res_pjsip/pjsip_transport_events.c
index c6c5d21af341786309e00efafbbcc8564dfd2413..aa575b7d584040bedfc588f4cfb8af28c87770b7 100644 (file)
--- 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 59a9fde5a5f6d9b4bcf191405cb3cb72e7dbed2c..b81057f0832f5de1e4d03b1f1d1c59672735f1a3 100644 (file)
--- 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);
 }