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 d0f754604a6e671c0a72e2d8ef693c092c1723fa..0111863219fb74c0ebfd0a4817699f174c58b8c6 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 69c256dab3cc39fda91af7b741898fc1c2bc7119..f26acb355b088a2fb804a177d3edf0d0edf9528a 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 bdee91fb30a53005cade37be1264f7f63ba39df7..985933e2dff9461c1d0e93d2359f5de7f9db1394 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 *);