[Asterisk-code-review] audiohook: Don't allow audiohooks to attach to hung up channels. (asterisk[16])

[Joshua Colp]

Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13961 )

Change subject: audiohook: Don't allow audiohooks to attach to hung up channels.
......................................................................

audiohook: Don't allow audiohooks to attach to hung up channels.

Given a scenario where MixMonitor was initiated over AMI it
was possible for the channel and MixMonitor thread to remain
alive past hang up of the channel. This scenario required
the AMI initiated MixMonitor to retrieve the channel, a
hangup to occur on the channel in another thread, and then
for MixMonitor to actually start. If this occurred the
MixMonitor thread would remain alive indefinitely and
the channel reference would remain.

This change ensures that audiohooks are never able to
be attached to channels that have been hung up. An
additional fix has also been done in app_mixmonitor to
properly release the channel reference if this occurs.

ASTERISK-28780

Change-Id: I8044c06daa06f0f16607788c596f55623be26f58
---
M apps/app_mixmonitor.c
M main/audiohook.c
2 files changed, 10 insertions(+), 0 deletions(-)

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



diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c
index a5c27bf..09148cc 100644
--- a/apps/app_mixmonitor.c
+++ b/apps/app_mixmonitor.c
@@ -987,6 +987,7 @@
 	if (startmon(chan, &mixmonitor->audiohook)) {
 		ast_log(LOG_WARNING, "Unable to add '%s' spy to channel '%s'\n",
 			mixmonitor_spy_type, ast_channel_name(chan));
+		ast_autochan_destroy(mixmonitor->autochan);
 		ast_audiohook_destroy(&mixmonitor->audiohook);
 		mixmonitor_free(mixmonitor);
 		return -1;
diff --git a/main/audiohook.c b/main/audiohook.c
index 4db32ea..ed16897 100644
--- a/main/audiohook.c
+++ b/main/audiohook.c
@@ -502,6 +502,15 @@
 {
 	ast_channel_lock(chan);
 
+	/* Don't allow an audiohook to be attached to a channel that is already hung up.
+	 * The hang up process is what actually notifies the audiohook that it should
+	 * stop.
+	 */
+	if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE)) {
+		ast_channel_unlock(chan);
+		return -1;
+	}
+
 	if (!ast_channel_audiohooks(chan)) {
 		struct ast_audiohook_list *ahlist;
 		/* Whoops... allocate a new structure */

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I8044c06daa06f0f16607788c596f55623be26f58
Gerrit-Change-Number: 13961
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200313/666de97a/attachment.html>