res_pjsip: Fix deadlock on reliable transport shutdown.

A deadlock can happen when the PJSIP monitor thread is shutting down a
connection oriented transport (TCP/TLS) used by a subscription at the same
time as another thread tries to send something for that subscription.  The
deadlock is between the pjsip monitor thread attempting to get the dialog
lock and another thread sending something for that dialog when it tries to
get the transport manager lock.

* res_pjsip_pubsub.c: Avoid the deadlock by pushing the subscription
removal to the subscription serializer.

* res_pjsip_registrar.c: Pushed off incoming registration contact removals
to a default serializer as a precaution.  Removing the contacts involves
sorcery access which in this case will involve database access.  Depending
upon the setup, the database may not be on the same machine and could take
awhile.  We don't want to hold up the pjsip monitor thread with
potentially long access times.

ASTERISK-27706

Change-Id: I56b647aea565f24dba33e9e5ebeed4cd3f31f8c4

diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index c1077f2dd8ba50e6127691aafc980448c854ea8c..8935bd1835925060cf1a8ebbd6898779141d021d 100644 (file)
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -833,6 +833,8 @@ static int reregister_immediately_cb(void *obj)
  *
  * \param obj What is needed to initiate a reregister attempt.
  *
+ * \note Normally executed by the pjsip monitor thread.
+ *
  * \return Nothing
  */
 static void registration_transport_shutdown_cb(void *obj)

diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index 9b45df54e887ded85d5ee4efcf03baa191753aeb..128982244a0c4d5a0ee33d292093a8d67f41b569 100644 (file)
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -560,15 +560,52 @@ static void *publication_resource_alloc(const char *name)
        return ast_sorcery_generic_alloc(sizeof(struct ast_sip_publication_resource), publication_resource_destroy);
 }
 
-static void sub_tree_transport_cb(void *data) {
+static int sub_tree_subscription_terminate_cb(void *data)
+{
        struct sip_subscription_tree *sub_tree = data;
 
-       ast_debug(3, "Transport destroyed.  Removing subscription '%s->%s'  prune on restart: %d\n",
+       if (!sub_tree->evsub) {
+               /* Something else already terminated the subscription. */
+               ao2_ref(sub_tree, -1);
+               return 0;
+       }
+
+       ast_debug(3, "Transport destroyed.  Removing subscription '%s->%s'  prune on boot: %d\n",
                sub_tree->persistence->endpoint, sub_tree->root->resource,
                sub_tree->persistence->prune_on_boot);
 
        sub_tree->state = SIP_SUB_TREE_TERMINATE_IN_PROGRESS;
        pjsip_evsub_terminate(sub_tree->evsub, PJ_TRUE);
+
+       ao2_ref(sub_tree, -1);
+       return 0;
+}
+
+/*!
+ * \internal
+ * \brief The reliable transport we used as a subscription contact has shutdown.
+ *
+ * \param data What subscription needs to be terminated.
+ *
+ * \note Normally executed by the pjsip monitor thread.
+ *
+ * \return Nothing
+ */
+static void sub_tree_transport_cb(void *data)
+{
+       struct sip_subscription_tree *sub_tree = data;
+
+       /*
+        * Push off the subscription termination to the serializer to
+        * avoid deadlock.  Another thread could be trying to send a
+        * message on the subscription that can deadlock with this
+        * thread.
+        */
+       ao2_ref(sub_tree, +1);
+       if (ast_sip_push_task(sub_tree->serializer, sub_tree_subscription_terminate_cb,
+               sub_tree)) {
+               ao2_ref(sub_tree, -1);
+       }
 }
 
 /*! \brief Destructor for subscription persistence */
@@ -621,7 +658,7 @@ static void subscription_persistence_update(struct sip_subscription_tree *sub_tr
                return;
        }
 
-       ast_debug(3, "Updating persistence for '%s->%s'  prune on restart: %s\n",
+       ast_debug(3, "Updating persistence for '%s->%s'  prune on boot: %s\n",
                sub_tree->persistence->endpoint, sub_tree->root->resource,
                sub_tree->persistence->prune_on_boot ? "yes" : "no");
 
@@ -645,7 +682,7 @@ static void subscription_persistence_update(struct sip_subscription_tree *sub_tr
                                                        sub_tree->endpoint, rdata);
 
                                        if (sub_tree->persistence->prune_on_boot) {
-                                               ast_debug(3, "adding transport monitor on %s for '%s->%s'  prune on restart: %d\n",
+                                               ast_debug(3, "adding transport monitor on %s for '%s->%s'  prune on boot: %d\n",
                                                        rdata->tp_info.transport->obj_name,
                                                        sub_tree->persistence->endpoint, sub_tree->root->resource,
                                                        sub_tree->persistence->prune_on_boot);

diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index 8f842884ef27ac4a3aab059337da98b9b5398832..675d177bb3d90c475482f1603ae42c3c7c2af8fa 100644 (file)
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -337,7 +337,7 @@ static int contact_transport_monitor_matcher(void *a, void *b)
                && strcmp(ma->contact_name, mb->contact_name) == 0;
 }
 
-static void register_contact_transport_shutdown_cb(void *data)
+static int register_contact_transport_remove_cb(void *data)
 {
        struct contact_transport_monitor *monitor = data;
        struct ast_sip_contact *contact;
@@ -345,7 +345,8 @@ static void register_contact_transport_shutdown_cb(void *data)
 
        aor = ast_sip_location_retrieve_aor(monitor->aor_name);
        if (!aor) {
-               return;
+               ao2_ref(monitor, -1);
+               return 0;
        }
 
        ao2_lock(aor);
@@ -365,6 +366,35 @@ static void register_contact_transport_shutdown_cb(void *data)
        }
        ao2_unlock(aor);
        ao2_ref(aor, -1);
+
+       ao2_ref(monitor, -1);
+       return 0;
+}
+
+/*!
+ * \internal
+ * \brief The reliable transport we registered as a contact has shutdown.
+ *
+ * \param data What contact needs to be removed.
+ *
+ * \note Normally executed by the pjsip monitor thread.
+ *
+ * \return Nothing
+ */
+static void register_contact_transport_shutdown_cb(void *data)
+{
+       struct contact_transport_monitor *monitor = data;
+
+       /*
+        * Push off to a default serializer.  This is in case sorcery
+        * does database accesses for contacts.  Database accesses may
+        * not be on this machine.  We don't want to tie up the pjsip
+        * monitor thread with potentially long access times.
+        */
+       ao2_ref(monitor, +1);
+       if (ast_sip_push_task(NULL, register_contact_transport_remove_cb, monitor)) {
+               ao2_ref(monitor, -1);
+       }
 }
 
 AST_VECTOR(excess_contact_vector, struct ast_sip_contact *);