[Asterisk-code-review] res_pjsip_mwi: potential double unref, and potential unwanted double ... (...asterisk[16])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/13046 )

Change subject: res_pjsip_mwi: potential double unref, and potential unwanted double link
......................................................................

res_pjsip_mwi: potential double unref, and potential unwanted double link

When creating an unsolicited MWI aggregate subscription it was possible for
the subscription object to be double unref'ed. This patch removes the explicit
unref as it is not needed since the RAII_VAR will handle it at function end.

Less concerning there was also a bug that could potentially allow the aggregate
subscription object to be added to the unsolicited container twice. This patch
ensures it is added only once.

ASTERISK-28575

Change-Id: I9ccfdb5ea788bc0c3618db183aae235e53c12763
---
M res/res_pjsip_mwi.c
1 file changed, 11 insertions(+), 4 deletions(-)

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



diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index 1fc614b..650c402 100644
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -1222,6 +1222,13 @@
 			if (!aggregate_sub) {
 				return 0; /* No MWI aggregation for you */
 			}
+
+			/*
+			 * Just in case we somehow get in the position of recreating with no previous
+			 * aggregate object, set recreate to false here in order to allow the new
+			 * object to be linked into the container below
+			 */
+			recreate = 0;
 		}
 	}
 
@@ -1265,13 +1272,13 @@
 
 	if (aggregate_sub) {
 		if (ao2_container_count(aggregate_sub->stasis_subs)) {
-			ao2_link_flags(unsolicited_mwi, aggregate_sub, OBJ_NOLOCK);
+			/* Only link if we're dealing with a new aggregate object */
+			if (!recreate) {
+				ao2_link_flags(unsolicited_mwi, aggregate_sub, OBJ_NOLOCK);
+			}
 			if (send_now && sub_added) {
 				send_notify(aggregate_sub, NULL, 0);
 			}
-		} else {
-			/* No stasis subscriptions then no MWI data to aggregate */
-			ao2_ref(aggregate_sub, -1);
 		}
 	}
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I9ccfdb5ea788bc0c3618db183aae235e53c12763
Gerrit-Change-Number: 13046
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191014/7b9c6f82/attachment.html>