[asterisk-commits] dvossel: branch 1.6.1 r230511 - in /branches/1.6.1: ./ apps/app_mixmonitor.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Nov 19 15:32:01 CST 2009


Author: dvossel
Date: Thu Nov 19 15:31:57 2009
New Revision: 230511

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=230511
Log:
Merged revisions 230509 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/trunk

................
  r230509 | dvossel | 2009-11-19 15:26:21 -0600 (Thu, 19 Nov 2009) | 17 lines
  
  Merged revisions 230508 via svnmerge from 
  https://origsvn.digium.com/svn/asterisk/branches/1.4
  
  ........
    r230508 | dvossel | 2009-11-19 15:22:46 -0600 (Thu, 19 Nov 2009) | 10 lines
    
    fixes MixMonitor thread not exiting when StopMixMonitor is used
    
    (closes issue #16152)
    Reported by: AlexMS
    Patches:
          stopmixmonitor_1.4.diff uploaded by dvossel (license 671)
    Tested by: dvossel, AlexMS
    
    Review: https://reviewboard.asterisk.org/r/424/
  ........
................

Modified:
    branches/1.6.1/   (props changed)
    branches/1.6.1/apps/app_mixmonitor.c

Propchange: branches/1.6.1/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.

Modified: branches/1.6.1/apps/app_mixmonitor.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.1/apps/app_mixmonitor.c?view=diff&rev=230511&r1=230510&r2=230511
==============================================================================
--- branches/1.6.1/apps/app_mixmonitor.c (original)
+++ branches/1.6.1/apps/app_mixmonitor.c Thu Nov 19 15:31:57 2009
@@ -140,18 +140,21 @@
 	 * immediately during stop_mixmonitor or channel destruction. */
 	int fs_quit;
 	struct ast_filestream *fs;
+	struct ast_audiohook *audiohook;
 };
 
+ /*!
+  * \internal
+  * \pre mixmonitor_ds must be locked before calling this function
+  */
 static void mixmonitor_ds_close_fs(struct mixmonitor_ds *mixmonitor_ds)
 {
-	ast_mutex_lock(&mixmonitor_ds->lock);
 	if (mixmonitor_ds->fs) {
 		ast_closestream(mixmonitor_ds->fs);
 		mixmonitor_ds->fs = NULL;
 		mixmonitor_ds->fs_quit = 1;
 		ast_verb(2, "MixMonitor close filestream\n");
 	}
-	ast_mutex_unlock(&mixmonitor_ds->lock);
 }
 
 static void mixmonitor_ds_destroy(void *data)
@@ -160,6 +163,7 @@
 
 	ast_mutex_lock(&mixmonitor_ds->lock);
 	mixmonitor_ds->chan = NULL;
+	mixmonitor_ds->audiohook = NULL;
 	mixmonitor_ds->destruction_ok = 1;
 	ast_cond_signal(&mixmonitor_ds->destruction_condition);
 	ast_mutex_unlock(&mixmonitor_ds->lock);
@@ -180,6 +184,20 @@
 	.chan_fixup = mixmonitor_ds_chan_fixup,
 };
 
+static void destroy_monitor_audiohook(struct mixmonitor *mixmonitor)
+{
+	if (mixmonitor->mixmonitor_ds) {
+		ast_mutex_lock(&mixmonitor->mixmonitor_ds->lock);
+		mixmonitor->mixmonitor_ds->audiohook = NULL;
+		ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
+	}
+	/* kill the audiohook.*/
+	ast_audiohook_lock(&mixmonitor->audiohook);
+	ast_audiohook_detach(&mixmonitor->audiohook);
+	ast_audiohook_unlock(&mixmonitor->audiohook);
+	ast_audiohook_destroy(&mixmonitor->audiohook);
+}
+
 static int startmon(struct ast_channel *chan, struct ast_audiohook *audiohook) 
 {
 	struct ast_channel *peer = NULL;
@@ -220,10 +238,10 @@
 
 	ast_verb(2, "Begin MixMonitor Recording %s\n", mixmonitor->name);
 
+	fs = &mixmonitor->mixmonitor_ds->fs;
+
+	/* The audiohook must enter and exit the loop locked */
 	ast_audiohook_lock(&mixmonitor->audiohook);
-
-	fs = &mixmonitor->mixmonitor_ds->fs;
-
 	while (mixmonitor->audiohook.status == AST_AUDIOHOOK_STATUS_RUNNING && !mixmonitor->mixmonitor_ds->fs_quit) {
 		struct ast_frame *fr = NULL;
 
@@ -234,6 +252,10 @@
 
 		if (!(fr = ast_audiohook_read_frame(&mixmonitor->audiohook, SAMPLES_PER_FRAME, AST_AUDIOHOOK_DIRECTION_BOTH, AST_FORMAT_SLINEAR)))
 			continue;
+
+		/* audiohook lock is not required for the next block.
+		 * Unlock it, but remember to lock it before looping or exiting */
+		ast_audiohook_unlock(&mixmonitor->audiohook);
 
 		ast_mutex_lock(&mixmonitor->mixmonitor_ds->lock);
 		if (!ast_test_flag(mixmonitor, MUXFLAG_BRIDGED) || (mixmonitor->mixmonitor_ds->chan && ast_bridged_channel(mixmonitor->mixmonitor_ds->chan))) {
@@ -266,25 +288,26 @@
 
 		/* All done! free it. */
 		ast_frame_free(fr, 0);
-	}
-
-	ast_audiohook_detach(&mixmonitor->audiohook);
+		ast_audiohook_lock(&mixmonitor->audiohook);
+	}
+
 	ast_audiohook_unlock(&mixmonitor->audiohook);
-	ast_audiohook_destroy(&mixmonitor->audiohook);
-
-
+
+	/* Datastore cleanup.  close the filestream and wait for ds destruction */
+	ast_mutex_lock(&mixmonitor->mixmonitor_ds->lock);
 	mixmonitor_ds_close_fs(mixmonitor->mixmonitor_ds);
+	if (!mixmonitor->mixmonitor_ds->destruction_ok) {
+		ast_cond_wait(&mixmonitor->mixmonitor_ds->destruction_condition, &mixmonitor->mixmonitor_ds->lock);
+	}
+	ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
+
+	/* kill the audiohook */
+	destroy_monitor_audiohook(mixmonitor);
 
 	if (mixmonitor->post_process) {
 		ast_verb(2, "Executing [%s]\n", mixmonitor->post_process);
 		ast_safe_system(mixmonitor->post_process);
 	}
-
-	ast_mutex_lock(&mixmonitor->mixmonitor_ds->lock);
-	if (!mixmonitor->mixmonitor_ds->destruction_ok) {
-		ast_cond_wait(&mixmonitor->mixmonitor_ds->destruction_condition, &mixmonitor->mixmonitor_ds->lock);
-	}
-	ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
 
 	ast_verb(2, "End MixMonitor Recording %s\n", mixmonitor->name);
 	mixmonitor_free(mixmonitor);
@@ -312,6 +335,7 @@
 
 	/* No need to lock mixmonitor_ds since this is still operating in the channel's thread */
 	mixmonitor_ds->chan = chan;
+	mixmonitor_ds->audiohook = &mixmonitor->audiohook;
 	datastore->data = mixmonitor_ds;
 
 	ast_channel_lock(chan);
@@ -353,6 +377,12 @@
 		return;
 	}
 
+	/* Setup the actual spy before creating our thread */
+	if (ast_audiohook_init(&mixmonitor->audiohook, AST_AUDIOHOOK_TYPE_SPY, mixmonitor_spy_type)) {
+		mixmonitor_free(mixmonitor);
+		return;
+	}
+
 	/* Copy over flags and channel name */
 	mixmonitor->flags = flags;
 	if (setup_mixmonitor_ds(mixmonitor, chan)) {
@@ -368,12 +398,6 @@
 
 	mixmonitor->filename = (char *) mixmonitor + sizeof(*mixmonitor) + strlen(chan->name) + 1;
 	strcpy(mixmonitor->filename, filename);
-
-	/* Setup the actual spy before creating our thread */
-	if (ast_audiohook_init(&mixmonitor->audiohook, AST_AUDIOHOOK_TYPE_SPY, mixmonitor_spy_type)) {
-		mixmonitor_free(mixmonitor);
-		return;
-	}
 
 	ast_set_flag(&mixmonitor->audiohook, AST_AUDIOHOOK_TRIGGER_SYNC);
 
@@ -478,13 +502,36 @@
 {
 	struct ast_datastore *datastore = NULL;
 
-	/* closing the filestream here guarantees the file is avaliable to the dialplan
-	 * after calling StopMixMonitor */
+	ast_channel_lock(chan);
+	ast_audiohook_detach_source(chan, mixmonitor_spy_type);
 	if ((datastore = ast_channel_datastore_find(chan, &mixmonitor_ds_info, NULL))) {
-		mixmonitor_ds_close_fs(datastore->data);
-	}
-
-	ast_audiohook_detach_source(chan, mixmonitor_spy_type);
+		struct mixmonitor_ds *mixmonitor_ds = datastore->data;
+
+		ast_mutex_lock(&mixmonitor_ds->lock);
+
+		/* closing the filestream here guarantees the file is avaliable to the dialplan
+	 	 * after calling StopMixMonitor */
+		mixmonitor_ds_close_fs(mixmonitor_ds);
+
+		/* The mixmonitor thread may be waiting on the audiohook trigger.
+		 * In order to exit from the mixmonitor loop before waiting on channel
+		 * destruction, poke the audiohook trigger. */
+		if (mixmonitor_ds->audiohook) {
+			ast_audiohook_lock(mixmonitor_ds->audiohook);
+			ast_cond_signal(&mixmonitor_ds->audiohook->trigger);
+			ast_audiohook_unlock(mixmonitor_ds->audiohook);
+			mixmonitor_ds->audiohook = NULL;
+		}
+
+		ast_mutex_unlock(&mixmonitor_ds->lock);
+
+		/* Remove the datastore so the monitor thread can exit */
+		if (!ast_channel_datastore_remove(chan, datastore)) {
+			ast_datastore_free(datastore);
+		}
+	}
+	ast_channel_unlock(chan);
+
 	return 0;
 }
 




More information about the asterisk-commits mailing list