[Asterisk-code-review] res_pjsip_pubsub: Postpone destruction of old subscriptions on RLS up... (asterisk[19])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19009 )

Change subject: res_pjsip_pubsub: Postpone destruction of old subscriptions on RLS update
......................................................................

res_pjsip_pubsub: Postpone destruction of old subscriptions on RLS update

Set termination state to old subscriptions to prevent queueing and sending
NOTIFY messages on exten/device state changes.

Postpone destruction of old subscriptions until all already queued tasks
that may be using old subscriptions have completed.

ASTERISK-29906

Change-Id: I96582aad3a26515ca73a8460ee6756f56f6ba23b
---
M res/res_pjsip_exten_state.c
M res/res_pjsip_pubsub.c
2 files changed, 47 insertions(+), 2 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit




diff --git a/res/res_pjsip_exten_state.c b/res/res_pjsip_exten_state.c
index b51df87..3c7baee 100644
--- a/res/res_pjsip_exten_state.c
+++ b/res/res_pjsip_exten_state.c
@@ -296,7 +296,8 @@
 		.body_data = &task_data->exten_state_data,
 	};
 
-	/* Terminated subscriptions are no longer associated with a valid tree, and sending
+	/* The subscription was terminated while notify_task was in queue.
+	   Terminated subscriptions are no longer associated with a valid tree, and sending
 	 * NOTIFY messages on a subscription which has already been terminated won't work.
 	 */
 	if (ast_sip_subscription_is_terminated(task_data->exten_state_sub->sip_sub)) {
@@ -339,6 +340,13 @@
 	struct notify_task_data *task_data;
 	struct exten_state_subscription *exten_state_sub = data;
 
+	/* Terminated subscriptions are no longer associated with a valid tree.
+	 * Do not queue notify_task.
+	 */
+	if (ast_sip_subscription_is_terminated(exten_state_sub->sip_sub)) {
+		return 0;
+	}
+
 	if (!(task_data = alloc_notify_task_data(exten, exten_state_sub, info))) {
 		return -1;
 	}
diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index d0f3717..4086445 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -3994,6 +3994,15 @@
 	return 0;
 }
 
+static int destroy_subscriptions_task(void *obj)
+{
+	struct ast_sip_subscription *sub = (struct ast_sip_subscription *) obj;
+
+	destroy_subscriptions(sub);
+
+	return 0;
+}
+
 /*!
  * \brief Called whenever an in-dialog SUBSCRIBE is received
  *
@@ -4067,8 +4076,19 @@
 							AST_SCHED_DEL_UNREF(sched, sub_tree->notify_sched_id, ao2_ref(sub_tree, -1));
 							sub_tree->send_scheduled_notify = 0;
 						}
+
+						/* Terminate old subscriptions to stop sending NOTIFY messages on exten/device state changes */
+						set_state_terminated(old_root);
+
+						/* Shutdown old subscriptions to remove exten/device state change callbacks
+						 that can queue tasks for old subscriptions */
 						shutdown_subscriptions(old_root);
-						destroy_subscriptions(old_root);
+
+						/* Postpone destruction until all already queued tasks that may be using old subscriptions have completed */
+						if (ast_sip_push_task(sub_tree->serializer, destroy_subscriptions_task, old_root)) {
+							ast_log(LOG_ERROR, "Failed to push task to destroy old subscriptions for RLS '%s->%s'.\n",
+								ast_sorcery_object_get_id(endpoint), old_root->resource);
+						}
 					} else {
 						destroy_subscriptions(new_root);
 					}

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19009
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 19
Gerrit-Change-Id: I96582aad3a26515ca73a8460ee6756f56f6ba23b
Gerrit-Change-Number: 19009
Gerrit-PatchSet: 2
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220909/7785d391/attachment-0001.html>