[Asterisk-code-review] stasis: Fix "FRACK!, Failed assertion bad magic number" when unsubscr... (asterisk[master])

[Joshua Colp]

Hello Evgenios Muratidis,

I'd like you to do a code review. Please visit

    https://gerrit.asterisk.org/c/asterisk/+/15932

to review the following change.


Change subject: stasis: Fix "FRACK!, Failed assertion bad magic number" when unsubscribing
......................................................................

stasis: Fix "FRACK!, Failed assertion bad magic number" when unsubscribing

When unsubscribing from an endpoint technology a FRACK
would occur due to incorrect reference counting. This fixes
that issue, along with some other issues.

Fixed a typo in get_subscription when calling ao2_find as it
needed to pass the endpoint ID and not the entire object.

Fixed scenario where a subscription would get returned when
it shouldn't have been when searching based on endpoint
technology.

A doulbe unreference has also been resolved by only explicitly
releasing the reference held by tech_subscriptions.

ASTERISK-28237 #close
Reported by: Lucas Tardioli Silveira

Change-Id: Ia91b15f8e5ea68f850c66889a6325d9575901729
---
M res/stasis/messaging.c
1 file changed, 12 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/32/15932/1

diff --git a/res/stasis/messaging.c b/res/stasis/messaging.c
index 2caa8ed..e1d54c4 100644
--- a/res/stasis/messaging.c
+++ b/res/stasis/messaging.c
@@ -396,7 +396,7 @@
 	struct message_subscription *sub = NULL;
 
 	if (endpoint && !ast_strlen_zero(ast_endpoint_get_resource(endpoint))) {
-		sub = ao2_find(endpoint_subscriptions, endpoint, OBJ_SEARCH_KEY);
+		sub = ao2_find(endpoint_subscriptions, ast_endpoint_get_id(endpoint), OBJ_SEARCH_KEY);
 	} else {
 		int i;
 
@@ -408,6 +408,11 @@
 				ao2_bump(sub);
 				break;
 			}
+
+			/* Need to reset the pointer at this line to prevent from using the wrong subscription due to
+			 * the token check failing.
+			 */
+			sub = NULL;
 		}
 		ast_rwlock_unlock(&tech_subscriptions_lock);
 	}
@@ -441,10 +446,10 @@
 			AST_VECTOR_REMOVE_CMP_UNORDERED(&tech_subscriptions, endpoint ? ast_endpoint_get_id(endpoint) : TECH_WILDCARD,
 				messaging_subscription_cmp, AST_VECTOR_ELEM_CLEANUP_NOOP);
 			ast_rwlock_unlock(&tech_subscriptions_lock);
+			ao2_ref(sub, -1); /* Release the reference held by tech_subscriptions */
 		}
 	}
 	ao2_unlock(sub);
-	ao2_ref(sub, -1);
 
 	ast_debug(3, "App '%s' unsubscribed to messages from endpoint '%s'\n", app_name, endpoint ? ast_endpoint_get_id(endpoint) : "-- ALL --");
 	ast_test_suite_event_notify("StasisMessagingSubscription", "SubState: Unsubscribed\r\nAppName: %s\r\nToken: %s\r\n",
@@ -464,6 +469,11 @@
 		return NULL;
 	}
 
+	/* Either endpoint_subscriptions or tech_subscriptions will hold a reference to
+	 * the subscription. This reference is released to allow the subscription to
+	 * eventually destruct when there are no longer any applications receiving
+	 * events from the subscription.
+	 */
 	if (endpoint && !ast_strlen_zero(ast_endpoint_get_resource(endpoint))) {
 		ao2_link(endpoint_subscriptions, sub);
 	} else {

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ia91b15f8e5ea68f850c66889a6325d9575901729
Gerrit-Change-Number: 15932
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Evgenios Muratidis <jone1984 at hotmail.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210524/bf8d1b9b/attachment.html>