Merge "res_pjsip_mwi: potential double unref, and potential unwanted double link"
[asterisk/asterisk.git] / res / res_pjsip / pjsip_transport_management.c
index efda37d..af572d3 100644 (file)
@@ -56,21 +56,22 @@ struct monitored_transport {
        int sip_received;
 };
 
-/*! \brief Callback function to send keepalive */
-static int keepalive_transport_cb(void *obj, void *arg, int flags)
+static void keepalive_transport_send_keepalive(struct monitored_transport *monitored)
 {
-       struct monitored_transport *monitored = obj;
        pjsip_tpselector selector = {
                .type = PJSIP_TPSELECTOR_TRANSPORT,
                .u.transport = monitored->transport,
        };
 
        pjsip_tpmgr_send_raw(pjsip_endpt_get_tpmgr(ast_sip_get_pjsip_endpoint()),
-               monitored->transport->key.type, &selector, NULL, keepalive_packet.ptr, keepalive_packet.slen,
-               &monitored->transport->key.rem_addr, pj_sockaddr_get_len(&monitored->transport->key.rem_addr),
+               monitored->transport->key.type,
+               &selector,
+               NULL,
+               keepalive_packet.ptr,
+               keepalive_packet.slen,
+               &monitored->transport->key.rem_addr,
+               pj_sockaddr_get_len(&monitored->transport->key.rem_addr),
                NULL, NULL);
-
-       return 0;
 }
 
 /*! \brief Thread which sends keepalives to all active connection-oriented transports */
@@ -90,12 +91,26 @@ static void *keepalive_transport_thread(void *data)
                return NULL;
        }
 
-       /* Once loaded this module just keeps on going as it is unsafe to stop and change the underlying
-        * callback for the transport manager.
+       /*
+        * Once loaded this module just keeps on going as it is unsafe to stop
+        * and change the underlying callback for the transport manager.
         */
        while (keepalive_interval) {
+               struct ao2_iterator iter;
+               struct monitored_transport *monitored;
+
                sleep(keepalive_interval);
-               ao2_callback(transports, OBJ_NODATA, keepalive_transport_cb, NULL);
+
+               /*
+                * We must use the iterator to avoid deadlock between the container lock
+                * and the pjproject transport manager group lock when sending
+                * the keepalive packet.
+                */
+               iter = ao2_iterator_init(transports, 0);
+               for (; (monitored = ao2_iterator_next(&iter)); ao2_ref(monitored, -1)) {
+                       keepalive_transport_send_keepalive(monitored);
+               }
+               ao2_iterator_destroy(&iter);
        }
 
        ao2_ref(transports, -1);
@@ -104,10 +119,8 @@ static void *keepalive_transport_thread(void *data)
 
 AST_THREADSTORAGE(desc_storage);
 
-static int idle_sched_cb(const void *data)
+static int idle_sched_init_pj_thread(void)
 {
-       struct monitored_transport *monitored = (struct monitored_transport *) data;
-
        if (!pj_thread_is_registered()) {
                pj_thread_t *thread;
                pj_thread_desc *desc;
@@ -115,8 +128,7 @@ static int idle_sched_cb(const void *data)
                desc = ast_threadstorage_get(&desc_storage, sizeof(pj_thread_desc));
                if (!desc) {
                        ast_log(LOG_ERROR, "Could not get thread desc from thread-local storage.\n");
-                       ao2_ref(monitored, -1);
-                       return 0;
+                       return -1;
                }
 
                pj_bzero(*desc, sizeof(*desc));
@@ -124,13 +136,65 @@ static int idle_sched_cb(const void *data)
                pj_thread_register("Transport Monitor", *desc, &thread);
        }
 
-       if (!monitored->sip_received) {
-               ast_log(LOG_NOTICE, "Shutting down transport '%s' since no request was received in %d seconds\n",
-                       monitored->transport->info, IDLE_TIMEOUT / 1000);
+       return 0;
+}
+
+static struct monitored_transport *get_monitored_transport_by_name(const char *obj_name)
+{
+       struct ao2_container *transports;
+       struct monitored_transport *monitored = NULL;
+
+       transports = ao2_global_obj_ref(monitored_transports);
+       if (transports) {
+               monitored = ao2_find(transports, obj_name, OBJ_SEARCH_KEY);
+       }
+       ao2_cleanup(transports);
+
+       /* Caller is responsible for cleaning up reference */
+       return monitored;
+}
+
+static int idle_sched_cb(const void *data)
+{
+       char *obj_name = (char *) data;
+       struct monitored_transport *monitored;
+
+       if (idle_sched_init_pj_thread()) {
+               ast_free(obj_name);
+               return 0;
+       }
+
+       monitored = get_monitored_transport_by_name(obj_name);
+       if (monitored) {
+               if (!monitored->sip_received) {
+                       ast_log(LOG_NOTICE, "Shutting down transport '%s' since no request was received in %d seconds\n",
+                               monitored->transport->info, IDLE_TIMEOUT / 1000);
+                       pjsip_transport_shutdown(monitored->transport);
+               }
+               ao2_ref(monitored, -1);
+       }
+
+       ast_free(obj_name);
+       return 0;
+}
+
+static int idle_sched_cleanup(const void *data)
+{
+       char *obj_name = (char *) data;
+       struct monitored_transport *monitored;
+
+       if (idle_sched_init_pj_thread()) {
+               ast_free(obj_name);
+               return 0;
+       }
+
+       monitored = get_monitored_transport_by_name(obj_name);
+       if (monitored) {
                pjsip_transport_shutdown(monitored->transport);
+               ao2_ref(monitored, -1);
        }
 
-       ao2_ref(monitored, -1);
+       ast_free(obj_name);
        return 0;
 }
 
@@ -167,13 +231,13 @@ static void monitored_transport_state_callback(pjsip_transport *transport, pjsip
                        ao2_link(transports, monitored);
 
                        if (transport->dir == PJSIP_TP_DIR_INCOMING) {
-                               /* Let the scheduler inherit the reference from allocation */
-                               if (ast_sched_add_variable(sched, IDLE_TIMEOUT, idle_sched_cb, monitored, 1) < 0) {
-                                       /* Uh Oh.  Could not schedule the idle check.  Kill the transport. */
+                               char *obj_name = ast_strdup(transport->obj_name);
+
+                               if (!obj_name
+                                  || ast_sched_add_variable(sched, IDLE_TIMEOUT, idle_sched_cb, obj_name, 1) < 0) {
+                                       /* Shut down the transport if anything fails */
                                        pjsip_transport_shutdown(transport);
-                               } else {
-                                       /* monitored ref successfully passed to idle_sched_cb() */
-                                       break;
+                                       ast_free(obj_name);
                                }
                        }
                        ao2_ref(monitored, -1);
@@ -288,23 +352,14 @@ static struct ast_sorcery_observer keepalive_global_observer = {
  */
 static pj_bool_t idle_monitor_on_rx_request(pjsip_rx_data *rdata)
 {
-       struct ao2_container *transports;
        struct monitored_transport *idle_trans;
 
-       transports = ao2_global_obj_ref(monitored_transports);
-       if (!transports) {
-               return PJ_FALSE;
-       }
-
-       idle_trans = ao2_find(transports, rdata->tp_info.transport->obj_name, OBJ_SEARCH_KEY);
-       ao2_ref(transports, -1);
-       if (!idle_trans) {
-               return PJ_FALSE;
+       idle_trans = get_monitored_transport_by_name(rdata->tp_info.transport->obj_name);
+       if (idle_trans) {
+               idle_trans->sip_received = 1;
+               ao2_ref(idle_trans, -1);
        }
 
-       idle_trans->sip_received = 1;
-       ao2_ref(idle_trans, -1);
-
        return PJ_FALSE;
 }
 
@@ -318,8 +373,8 @@ int ast_sip_initialize_transport_management(void)
 {
        struct ao2_container *transports;
 
-       transports = ao2_container_alloc(TRANSPORTS_BUCKETS, monitored_transport_hash_fn,
-               monitored_transport_cmp_fn);
+       transports = ao2_container_alloc_hash(AO2_ALLOC_OPT_LOCK_MUTEX, 0, TRANSPORTS_BUCKETS,
+               monitored_transport_hash_fn, NULL, monitored_transport_cmp_fn);
        if (!transports) {
                ast_log(LOG_ERROR, "Could not create container for transports to perform keepalive on.\n");
                return AST_MODULE_LOAD_DECLINE;
@@ -369,6 +424,7 @@ void ast_sip_destroy_transport_management(void)
 
        ast_sip_unregister_service(&idle_monitor_module);
 
+       ast_sched_clean_by_callback(sched, idle_sched_cb, idle_sched_cleanup);
        ast_sched_context_destroy(sched);
        sched = NULL;