[Asterisk-code-review] res pjsip pubsub.c: Use distributor serializer for incoming ... (asterisk[certified/13.8])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: res_pjsip_pubsub.c: Use distributor serializer for incoming subscriptions.
......................................................................


res_pjsip_pubsub.c: Use distributor serializer for incoming subscriptions.

We must continue using the serializer that the original SUBSCRIBE came in
on for the dialog.  There may be retransmissions already enqueued in the
original serializer that can result in reentrancy and message sequencing
problems.  The "sip_transaction Unable to register SUBSCRIBE transaction
(key exists)" message is a notable symptom of this issue.

Outgoing subscriptions still create the pjsip/pubsub/<endpoint>
serializers for their dialogs.

ASTERISK-26088
Reported by:  Richard Mudgett

Change-Id: I18b00bb74a56747b2c8c29543a82440b110bf0b0
---
M res/res_pjsip_pubsub.c
1 file changed, 20 insertions(+), 8 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index d859b76..c0dd348 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -1229,10 +1229,9 @@
 	pjsip_dlg_inc_session(dlg, &pubsub_module);
 }
 
-static struct sip_subscription_tree *allocate_subscription_tree(struct ast_sip_endpoint *endpoint)
+static struct sip_subscription_tree *allocate_subscription_tree(struct ast_sip_endpoint *endpoint, pjsip_rx_data *rdata)
 {
 	struct sip_subscription_tree *sub_tree;
-	char tps_name[AST_TASKPROCESSOR_MAX_NAME + 1];
 
 	sub_tree = ao2_alloc(sizeof *sub_tree, subscription_tree_destructor);
 	if (!sub_tree) {
@@ -1241,11 +1240,24 @@
 
 	ast_module_ref(ast_module_info->self);
 
-	/* Create name with seq number appended. */
-	ast_taskprocessor_build_name(tps_name, sizeof(tps_name), "pjsip/pubsub/%s",
-		ast_sorcery_object_get_id(endpoint));
+	if (rdata) {
+		/*
+		 * We must continue using the serializer that the original
+		 * SUBSCRIBE came in on for the dialog.  There may be
+		 * retransmissions already enqueued in the original
+		 * serializer that can result in reentrancy and message
+		 * sequencing problems.
+		 */
+		sub_tree->serializer = ast_sip_get_distributor_serializer(rdata);
+	} else {
+		char tps_name[AST_TASKPROCESSOR_MAX_NAME + 1];
 
-	sub_tree->serializer = ast_sip_create_serializer_named(tps_name);
+		/* Create name with seq number appended. */
+		ast_taskprocessor_build_name(tps_name, sizeof(tps_name), "pjsip/pubsub/%s",
+			ast_sorcery_object_get_id(endpoint));
+
+		sub_tree->serializer = ast_sip_create_serializer_named(tps_name);
+	}
 	if (!sub_tree->serializer) {
 		ao2_ref(sub_tree, -1);
 		return NULL;
@@ -1286,7 +1298,7 @@
 	pjsip_dialog *dlg;
 	struct subscription_persistence *persistence;
 
-	sub_tree = allocate_subscription_tree(endpoint);
+	sub_tree = allocate_subscription_tree(endpoint, rdata);
 	if (!sub_tree) {
 		*dlg_status = PJ_ENOMEM;
 		return NULL;
@@ -1593,7 +1605,7 @@
 	pjsip_evsub *evsub;
 	struct sip_subscription_tree *sub_tree = NULL;
 
-	sub_tree = allocate_subscription_tree(endpoint);
+	sub_tree = allocate_subscription_tree(endpoint, NULL);
 	if (!sub_tree) {
 		return NULL;
 	}

-- 
To view, visit https://gerrit.asterisk.org/2995
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I18b00bb74a56747b2c8c29543a82440b110bf0b0
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.8
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>