Merge "res_pjsip: Fix deadlock on reliable transport shutdown."

3 files changed, 75 insertions, 6 deletions

diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 8a90849c0..0d815ad39 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -834,6 +834,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 9e0718f51..d98491495 100644
--- 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 bdee91fb3..985933e2d 100644
--- 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 *);