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 03e08d5c33c60b3e099feb322161bec85c157413)

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 db5feefb0662631a045813c43a9c1122861cb78e..31aaa59814ca44d9db81b50d36b3909f9e609e82 100644 (file)
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -1117,6 +1117,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);
@@ -1130,8 +1131,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);
 }