[asterisk-commits] ARI: Ensure announcer channels are destroyed. (asterisk[13])

[SVN commits to the Asterisk project]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: ARI: Ensure announcer channels are destroyed.
......................................................................


ARI: Ensure announcer channels are destroyed.

Announcer channels were not being destroyed because the
stasis_app_control structure that referenced them was not being
destroyed. The control structure was not being destroyed because it was
not being unlinked from its container. It was not being unlinked from
its container because the after bridge callback for the announcer
channel was not being run. The after bridge callback was not being run
because the after bridge datastore was not being removed from the
channel on destruction. The channel was not being destroyed because the
hangup that used to destroy the channel was now only reducing the
reference count to one. The reference count of the channel was only
being reduced to one because the stasis_app_control structure was
holding the final reference...

The control structure used to not keep a reference to the channel, so
that loop described above did not happen.

The solution is to manually remove the control structure from its
container when the playback on a bridge is complete.

ASTERISK-26083 #close
Reported by Joshua Colp

Change-Id: I0ddc0f64484ea0016245800b409b567dfe85cfb4
---
M include/asterisk/stasis_app.h
M res/ari/resource_bridges.c
M res/res_stasis.c
3 files changed, 33 insertions(+), 1 deletion(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/include/asterisk/stasis_app.h b/include/asterisk/stasis_app.h
index 90ef82e..8ceeffb 100644
--- a/include/asterisk/stasis_app.h
+++ b/include/asterisk/stasis_app.h
@@ -746,6 +746,15 @@
 	struct stasis_app_control *control);
 
 /*!
+ * \brief remove channel from list of ARI playback channels for bridges.
+ *
+ * \param bridge_id The unique ID of the bridge the playback channel is in.
+ * \param control The app control structure for the playback channel
+ */
+void stasis_app_bridge_playback_channel_remove(char *bridge_id,
+	struct stasis_app_control *control);
+
+/*!
  * \brief Result codes used when adding/removing channels to/from bridges.
  */
 enum stasis_app_control_channel_result {
diff --git a/res/ari/resource_bridges.c b/res/ari/resource_bridges.c
index a86f312..39709d0 100644
--- a/res/ari/resource_bridges.c
+++ b/res/ari/resource_bridges.c
@@ -278,6 +278,7 @@
 	struct ast_channel *bridge_channel;
 	struct stasis_app_control *control;
 	struct stasis_forward *forward;
+	char bridge_id[0];
 };
 
 static void *bridge_channel_control_thread(void *data)
@@ -286,6 +287,7 @@
 	struct ast_channel *bridge_channel = thread_data->bridge_channel;
 	struct stasis_app_control *control = thread_data->control;
 	struct stasis_forward *forward = thread_data->forward;
+	char *bridge_id = ast_strdupa(thread_data->bridge_id);
 
 	RAII_VAR(struct ast_callid *, callid, ast_channel_callid(bridge_channel), ast_callid_cleanup);
 
@@ -299,6 +301,7 @@
 	stasis_app_control_execute_until_exhausted(bridge_channel, control);
 	stasis_app_control_flush_queue(control);
 
+	stasis_app_bridge_playback_channel_remove(bridge_id, control);
 	stasis_forward_cancel(forward);
 	ao2_cleanup(control);
 	ast_hangup(bridge_channel);
@@ -464,8 +467,9 @@
 	}
 
 	/* Give play_channel and control reference to the thread data */
-	thread_data = ast_calloc(1, sizeof(*thread_data));
+	thread_data = ast_malloc(sizeof(*thread_data) + strlen(bridge->uniqueid) + 1);
 	if (!thread_data) {
+		stasis_app_bridge_playback_channel_remove((char *)bridge->uniqueid, control);
 		ast_ari_response_alloc_failed(response);
 		return;
 	}
@@ -473,8 +477,11 @@
 	thread_data->bridge_channel = play_channel;
 	thread_data->control = control;
 	thread_data->forward = channel_forward;
+	/* Safe */
+	strcpy(thread_data->bridge_id, bridge->uniqueid);
 
 	if (ast_pthread_create_detached(&threadid, NULL, bridge_channel_control_thread, thread_data)) {
+		stasis_app_bridge_playback_channel_remove((char *)bridge->uniqueid, control);
 		ast_ari_response_alloc_failed(response);
 		ast_free(thread_data);
 		return;
diff --git a/res/res_stasis.c b/res/res_stasis.c
index fae9aa2..11aeb43 100644
--- a/res/res_stasis.c
+++ b/res/res_stasis.c
@@ -713,6 +713,22 @@
 	return 0;
 }
 
+void stasis_app_bridge_playback_channel_remove(char *bridge_id,
+	struct stasis_app_control *control)
+{
+	struct stasis_app_bridge_channel_wrapper *wrapper;
+
+	wrapper = ao2_find(app_bridges_playback, bridge_id, OBJ_SEARCH_KEY | OBJ_UNLINK);
+	if (wrapper) {
+		/* If wrapper is not found, then that means the after bridge callback has been
+		 * called or is in progress. No need to unlink the control here since that has
+		 * been done or is about to be done in the after bridge callback
+		 */
+		ao2_unlink(app_controls, control);
+		ao2_ref(wrapper, -1);
+	}
+}
+
 struct ast_channel *stasis_app_bridge_playback_channel_find(struct ast_bridge *bridge)
 {
 	struct stasis_app_bridge_channel_wrapper *playback_wrapper;

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0ddc0f64484ea0016245800b409b567dfe85cfb4
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>