[Asterisk-code-review] Fix deadlock handling subscribe req during res parking reload (asterisk[master])

[George Joseph]

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

Change subject: Fix deadlock handling subscribe req during res_parking reload
......................................................................

Fix deadlock handling subscribe req during res_parking reload

Split destroy_hint method to separate hint removal and extension hint
state changed callback, the latter now called via stasis.
This avoids deadlock between res_parking reload that is removing the
parking lot and the related hint and subscribe requests coming for the
same parking lot.

ASTERISK-28173

Change-Id: I5b03c3455b3b12b6f83cea4cc34f4b4b20444f7e
---
M main/pbx.c
1 file changed, 55 insertions(+), 30 deletions(-)

Approvals:
  Friendly Automation: Verified
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/main/pbx.c b/main/pbx.c
index 9e19f35..ee5ed7e 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -346,6 +346,7 @@
 };
 
 STASIS_MESSAGE_TYPE_DEFN_LOCAL(hint_change_message_type);
+STASIS_MESSAGE_TYPE_DEFN_LOCAL(hint_remove_message_type);
 
 #define HINTDEVICE_DATA_LENGTH 16
 AST_THREADSTORAGE(hintdevice_data);
@@ -3570,6 +3571,30 @@
 		return;
 	}
 
+	if (hint_remove_message_type() == stasis_message_type(msg)) {
+		/* The extension has already been destroyed */
+		struct ast_state_cb *state_cb;
+		struct ao2_iterator cb_iter;
+		struct ast_hint *hint = stasis_message_data(msg);
+
+		ao2_lock(hint);
+		hint->laststate = AST_EXTENSION_DEACTIVATED;
+		ao2_unlock(hint);
+
+		cb_iter = ao2_iterator_init(hint->callbacks, 0);
+		for (; (state_cb = ao2_iterator_next(&cb_iter)); ao2_ref(state_cb, -1)) {
+			execute_state_callback(state_cb->change_cb,
+			        hint->context_name,
+			        hint->exten_name,
+			        state_cb->data,
+			        AST_HINT_UPDATE_DEVICE,
+			        hint,
+			        NULL);
+		}
+		ao2_iterator_destroy(&cb_iter);
+		return;
+	}
+
 	if (ast_device_state_message_type() != stasis_message_type(msg)) {
 		return;
 	}
@@ -3855,34 +3880,7 @@
 	struct ast_hint *hint = obj;
 	int i;
 
-	if (hint->callbacks) {
-		struct ast_state_cb *state_cb;
-		const char *context_name;
-		const char *exten_name;
-
-		if (hint->exten) {
-			context_name = ast_get_context_name(ast_get_extension_context(hint->exten));
-			exten_name = ast_get_extension_name(hint->exten);
-			hint->exten = NULL;
-		} else {
-			/* The extension has already been destroyed */
-			context_name = hint->context_name;
-			exten_name = hint->exten_name;
-		}
-		hint->laststate = AST_EXTENSION_DEACTIVATED;
-		while ((state_cb = ao2_callback(hint->callbacks, OBJ_UNLINK, NULL, NULL))) {
-			/* Notify with -1 and remove all callbacks */
-			execute_state_callback(state_cb->change_cb,
-				context_name,
-				exten_name,
-				state_cb->data,
-				AST_HINT_UPDATE_DEVICE,
-				hint,
-				NULL);
-			ao2_ref(state_cb, -1);
-		}
-		ao2_ref(hint->callbacks, -1);
-	}
+	ao2_cleanup(hint->callbacks);
 
 	for (i = 0; i < AST_VECTOR_SIZE(&hint->devices); i++) {
 		char *device = AST_VECTOR_GET(&hint->devices, i);
@@ -3893,6 +3891,27 @@
 	ast_free(hint->last_presence_message);
 }
 
+/*! \brief Publish a hint removed event  */
+static int publish_hint_remove(struct ast_hint *hint)
+{
+	struct stasis_message *message;
+
+	if (!hint_remove_message_type()) {
+		return -1;
+	}
+
+	if (!(message = stasis_message_create(hint_remove_message_type(), hint))) {
+		ao2_ref(hint, -1);
+		return -1;
+	}
+
+	stasis_publish(ast_device_state_topic_all(), message);
+
+	ao2_ref(message, -1);
+
+	return 0;
+}
+
 /*! \brief Remove hint from extension */
 static int ast_remove_hint(struct ast_exten *e)
 {
@@ -3923,6 +3942,8 @@
 	hint->exten = NULL;
 	ao2_unlock(hint);
 
+	publish_hint_remove(hint);
+
 	ao2_ref(hint, -1);
 
 	return 0;
@@ -4029,8 +4050,7 @@
 		return -1;
 	}
 
-	/* The message is going to be published to two topics so the hint needs two refs */
-	if (!(message = stasis_message_create(hint_change_message_type(), ao2_bump(hint)))) {
+	if (!(message = stasis_message_create(hint_change_message_type(), hint))) {
 		ao2_ref(hint, -1);
 		return -1;
 	}
@@ -8418,6 +8438,7 @@
 	}
 	stasis_subscription_accept_message_type(device_state_sub, ast_device_state_message_type());
 	stasis_subscription_accept_message_type(device_state_sub, hint_change_message_type());
+	stasis_subscription_accept_message_type(device_state_sub, hint_remove_message_type());
 	stasis_subscription_set_filter(device_state_sub, STASIS_SUBSCRIPTION_FILTER_SELECTIVE);
 
 	if (!(presence_state_sub = stasis_subscribe(ast_presence_state_topic_all(), presence_state_cb, NULL))) {
@@ -8841,6 +8862,7 @@
 static void pbx_shutdown(void)
 {
 	STASIS_MESSAGE_TYPE_CLEANUP(hint_change_message_type);
+	STASIS_MESSAGE_TYPE_CLEANUP(hint_remove_message_type);
 
 	if (hints) {
 		ao2_container_unregister("hints");
@@ -8938,6 +8960,9 @@
 	if (STASIS_MESSAGE_TYPE_INIT(hint_change_message_type) != 0) {
 		return -1;
 	}
+	if (STASIS_MESSAGE_TYPE_INIT(hint_remove_message_type) != 0) {
+		return -1;
+	}
 
 	return (hints && hintdevices && autohints && statecbs) ? 0 : -1;
 }

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5b03c3455b3b12b6f83cea4cc34f4b4b20444f7e
Gerrit-Change-Number: 10959
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation (1000185)
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Giuseppe Sucameli <sucameli at netresults.it>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190205/8354fde5/attachment.html>