[asterisk-commits] dvossel: branch 1.4 r201423 - /branches/1.4/apps/app_mixmonitor.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed Jun 17 14:28:35 CDT 2009
Author: dvossel
Date: Wed Jun 17 14:28:12 2009
New Revision: 201423
URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=201423
Log:
StopMixMonitor race condition (not giving up file immediately)
StopMixMonitor only indicates to the MixMonitor thread to stop
writing to the file. It does not guarantee that the recording's
file handle is available to the dialplan immediately after execution.
This results in a race condition. To resolve this, the filestream
pointer is placed in a datastore on the channel. When StopMixMonitor
is called, the datastore is retrieved from the channel and the
filestream is closed immediately before returning to the dialplan.
Documentation indicating the use of StopMixMonitor to free files
has been updated as well.
(closes issue #15259)
Reported by: travisghansen
Tested by: dvossel
Review: https://reviewboard.asterisk.org/r/283/
Modified:
branches/1.4/apps/app_mixmonitor.c
Modified: branches/1.4/apps/app_mixmonitor.c
URL: http://svn.asterisk.org/svn-view/asterisk/branches/1.4/apps/app_mixmonitor.c?view=diff&rev=201423&r1=201422&r2=201423
==============================================================================
--- branches/1.4/apps/app_mixmonitor.c (original)
+++ branches/1.4/apps/app_mixmonitor.c Wed Jun 17 14:28:12 2009
@@ -64,7 +64,8 @@
"Records the audio on the current channel to the specified file.\n"
"If the filename is an absolute path, uses that path, otherwise\n"
"creates the file in the configured monitoring directory from\n"
-"asterisk.conf.\n\n"
+"asterisk.conf. Use of StopMixMonitor is required to guarantee\n"
+"the audio file is available for processing during dialplan execution.\n\n"
"Valid options:\n"
" a - Append to the file instead of overwriting it.\n"
" b - Only save audio to the file while the channel is bridged.\n"
@@ -88,8 +89,7 @@
static const char *stop_synopsis = "Stop recording a call through MixMonitor";
static const char *stop_desc = ""
" StopMixMonitor()\n\n"
-"Stops the audio recording that was started with a call to MixMonitor()\n"
-"on the current channel.\n"
+"Stop recording a call through MixMonitor, and free the recording's file handle.\n"
"";
struct module_symbols *me;
@@ -144,7 +144,25 @@
unsigned int destruction_ok;
ast_cond_t destruction_condition;
ast_mutex_t lock;
+
+ /* The filestream is held in the datastore so it can be stopped
+ * immediately during stop_mixmonitor or channel destruction. */
+ int fs_quit;
+ struct ast_filestream *fs;
};
+
+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;
+ if (option_verbose > 1)
+ ast_verbose(VERBOSE_PREFIX_2 "MixMonitor close filestream\n");
+ }
+ ast_mutex_unlock(&mixmonitor_ds->lock);
+}
static void mixmonitor_ds_destroy(void *data)
{
@@ -190,27 +208,41 @@
#define SAMPLES_PER_FRAME 160
+static void mixmonitor_free(struct mixmonitor *mixmonitor)
+{
+ if (mixmonitor) {
+ if (mixmonitor->mixmonitor_ds) {
+ ast_mutex_destroy(&mixmonitor->mixmonitor_ds->lock);
+ ast_cond_destroy(&mixmonitor->mixmonitor_ds->destruction_condition);
+ ast_free(mixmonitor->mixmonitor_ds);
+ }
+ ast_free(mixmonitor);
+ }
+}
+
static void *mixmonitor_thread(void *obj)
{
struct mixmonitor *mixmonitor = obj;
- struct ast_filestream *fs = NULL;
+ struct ast_filestream **fs = NULL;
unsigned int oflags;
char *ext;
int errflag = 0;
if (option_verbose > 1)
ast_verbose(VERBOSE_PREFIX_2 "Begin MixMonitor Recording %s\n", mixmonitor->name);
-
+
ast_audiohook_lock(&mixmonitor->audiohook);
- while (mixmonitor->audiohook.status == AST_AUDIOHOOK_STATUS_RUNNING) {
+ fs = &mixmonitor->mixmonitor_ds->fs;
+
+ while (mixmonitor->audiohook.status == AST_AUDIOHOOK_STATUS_RUNNING && !mixmonitor->mixmonitor_ds->fs_quit) {
struct ast_frame *fr = NULL;
-
+
ast_audiohook_trigger_wait(&mixmonitor->audiohook);
-
+
if (mixmonitor->audiohook.status != AST_AUDIOHOOK_STATUS_RUNNING)
break;
-
+
if (!(fr = ast_audiohook_read_frame(&mixmonitor->audiohook, SAMPLES_PER_FRAME, AST_AUDIOHOOK_DIRECTION_BOTH, AST_FORMAT_SLINEAR)))
continue;
@@ -218,32 +250,31 @@
if (!ast_test_flag(mixmonitor, MUXFLAG_BRIDGED) || (mixmonitor->mixmonitor_ds->chan && ast_bridged_channel(mixmonitor->mixmonitor_ds->chan))) {
ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
/* Initialize the file if not already done so */
- if (!fs && !errflag) {
+ if (!*fs && !errflag && !mixmonitor->mixmonitor_ds->fs_quit) {
oflags = O_CREAT | O_WRONLY;
oflags |= ast_test_flag(mixmonitor, MUXFLAG_APPEND) ? O_APPEND : O_TRUNC;
-
+
if ((ext = strrchr(mixmonitor->filename, '.')))
*(ext++) = '\0';
else
ext = "raw";
-
- if (!(fs = ast_writefile(mixmonitor->filename, ext, NULL, oflags, 0, 0644))) {
+
+ if (!(*fs = ast_writefile(mixmonitor->filename, ext, NULL, oflags, 0, 0644))) {
ast_log(LOG_ERROR, "Cannot open %s.%s\n", mixmonitor->filename, ext);
errflag = 1;
}
}
/* Write out the frame(s) */
- if (fs) {
+ if (*fs) {
struct ast_frame *cur;
for (cur = fr; cur; cur = AST_LIST_NEXT(cur, frame_list)) {
- ast_writestream(fs, cur);
+ ast_writestream(*fs, cur);
}
}
- } else {
- ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
}
+ ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
/* All done! free it. */
ast_frame_free(fr, 0);
@@ -252,12 +283,8 @@
ast_audiohook_detach(&mixmonitor->audiohook);
ast_audiohook_unlock(&mixmonitor->audiohook);
ast_audiohook_destroy(&mixmonitor->audiohook);
-
- if (option_verbose > 1)
- ast_verbose(VERBOSE_PREFIX_2 "End MixMonitor Recording %s\n", mixmonitor->name);
-
- if (fs)
- ast_closestream(fs);
+
+ mixmonitor_ds_close_fs(mixmonitor->mixmonitor_ds);
if (mixmonitor->post_process) {
if (option_verbose > 2)
@@ -270,10 +297,11 @@
ast_cond_wait(&mixmonitor->mixmonitor_ds->destruction_condition, &mixmonitor->mixmonitor_ds->lock);
}
ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
- ast_mutex_destroy(&mixmonitor->mixmonitor_ds->lock);
- ast_cond_destroy(&mixmonitor->mixmonitor_ds->destruction_condition);
- ast_free(mixmonitor->mixmonitor_ds);
- free(mixmonitor);
+
+ if (option_verbose > 1)
+ ast_verbose(VERBOSE_PREFIX_2 "End MixMonitor Recording %s\n", mixmonitor->name);
+
+ mixmonitor_free(mixmonitor);
return NULL;
}
@@ -291,6 +319,8 @@
ast_cond_init(&mixmonitor_ds->destruction_condition, NULL);
if (!(datastore = ast_channel_datastore_alloc(&mixmonitor_ds_info, NULL))) {
+ ast_mutex_destroy(&mixmonitor_ds->lock);
+ ast_cond_destroy(&mixmonitor_ds->destruction_condition);
ast_free(mixmonitor_ds);
return -1;
}
@@ -342,6 +372,7 @@
/* Copy over flags and channel name */
mixmonitor->flags = flags;
if (setup_mixmonitor_ds(mixmonitor, chan)) {
+ mixmonitor_free(mixmonitor);
return;
}
mixmonitor->name = (char *) mixmonitor + sizeof(*mixmonitor);
@@ -467,6 +498,13 @@
static int stop_mixmonitor_exec(struct ast_channel *chan, void *data)
{
struct ast_module_user *u;
+ struct ast_datastore *datastore = NULL;
+
+ /* closing the filestream here guarantees the file is avaliable to the dialplan
+ * after calling StopMixMonitor */
+ if ((datastore = ast_channel_datastore_find(chan, &mixmonitor_ds_info, NULL))) {
+ mixmonitor_ds_close_fs(datastore->data);
+ }
u = ast_module_user_add(chan);
More information about the asterisk-commits
mailing list