[Asterisk-code-review] app queue: Fix extension state subscriptions removed on dial... (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/7540 )

Change subject: app_queue: Fix extension state subscriptions removed on dialplan reload
......................................................................

app_queue: Fix extension state subscriptions removed on dialplan reload

The approach with having a single global subscription to all extension
state changes has one issue: dynamically created hints don't have any
watchers and are therefore garbage collected on the first dialplan
reload.

This change creates a state subscription for every queue member with a
hint as state_interface, thus increasing the count of watches for
hints, so they are not destroyed prematurely anymore.

There are 2 side effects:
1. The state change callback in app_queue is not executed when
   there are no members referring to the extension.
2. The callback is called multiple times for the same hint if it's
   associated with more than one queue member.

Reported by: Steven T. Wheeler

ASTERISK-18411 #close

Change-Id: I4956af2136ea2a7f110ac9272eae5f6e676d8f89
---
M apps/app_queue.c
1 file changed, 15 insertions(+), 5 deletions(-)

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



diff --git a/apps/app_queue.c b/apps/app_queue.c
index 801695f..a140847 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -1575,6 +1575,7 @@
 	char state_exten[AST_MAX_EXTENSION]; /*!< Extension to get state from (if using hint) */
 	char state_context[AST_MAX_CONTEXT]; /*!< Context to use when getting state (if using hint) */
 	char state_interface[AST_CHANNEL_NAME]; /*!< Technology/Location from which to read devicestate changes */
+	int state_id;                        /*!< Extension state callback id (if using hint) */
 	char membername[80];                 /*!< Member name to use in queue logs */
 	int penalty;                         /*!< Are we a last resort? */
 	int calls;                           /*!< Number of calls serviced by this member */
@@ -2632,12 +2633,21 @@
 	return ast_strlen_zero(cur->state_exten) ? ast_device_state(cur->state_interface) : extensionstate2devicestate(ast_extension_state(NULL, cur->state_context, cur->state_exten));
 }
 
+static void destroy_queue_member_cb(void *obj)
+{
+	struct member *mem = obj;
+
+	if (mem->state_id != -1) {
+		ast_extension_state_del(mem->state_id, extension_state_cb);
+	}
+}
+
 /*! \brief allocate space for new queue member and set fields based on parameters passed */
 static struct member *create_queue_member(const char *interface, const char *membername, int penalty, int paused, const char *state_interface, int ringinuse)
 {
 	struct member *cur;
 
-	if ((cur = ao2_alloc(sizeof(*cur), NULL))) {
+	if ((cur = ao2_alloc(sizeof(*cur), destroy_queue_member_cb))) {
 		cur->ringinuse = ringinuse;
 		cur->penalty = penalty;
 		cur->paused = paused;
@@ -2664,6 +2674,10 @@
 
 			ast_copy_string(cur->state_exten, exten, sizeof(cur->state_exten));
 			ast_copy_string(cur->state_context, S_OR(context, "default"), sizeof(cur->state_context));
+
+			cur->state_id = ast_extension_state_add(cur->state_context, cur->state_exten, extension_state_cb, NULL);
+		} else {
+			cur->state_id = -1;
 		}
 		cur->status = get_queue_member_status(cur);
 	}
@@ -11084,8 +11098,6 @@
 
 	device_state_sub = stasis_unsubscribe_and_join(device_state_sub);
 
-	ast_extension_state_del(0, extension_state_cb);
-
 	ast_unload_realtime("queue_members");
 	ao2_cleanup(queues);
 	ao2_cleanup(pending_members);
@@ -11242,8 +11254,6 @@
 	err |= STASIS_MESSAGE_TYPE_INIT(queue_agent_complete_type);
 	err |= STASIS_MESSAGE_TYPE_INIT(queue_agent_dump_type);
 	err |= STASIS_MESSAGE_TYPE_INIT(queue_agent_ringnoanswer_type);
-
-	ast_extension_state_add(NULL, NULL, extension_state_cb, NULL);
 
 	if (err) {
 		unload_module();

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4956af2136ea2a7f110ac9272eae5f6e676d8f89
Gerrit-Change-Number: 7540
Gerrit-PatchSet: 1
Gerrit-Owner: Ivan Poddubny <ivan.poddubny at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171215/704772f2/attachment.html>