[asterisk-commits] mmichelson: branch 1.6.0 r173590 - in /branches/1.6.0: ./ apps/app_mixmonitor.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Feb 5 12:39:35 CST 2009
Author: mmichelson
Date: Thu Feb 5 12:39:35 2009
New Revision: 173590
URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=173590
Log:
Merged revisions 173589 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk
................
r173589 | mmichelson | 2009-02-05 12:34:06 -0600 (Thu, 05 Feb 2009) | 33 lines
Merged revisions 173559 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r173559 | mmichelson | 2009-02-05 11:34:33 -0600 (Thu, 05 Feb 2009) | 25 lines
Fix a problem where a channel pointer becomes invalid due to masquerading or hanging up.
app_mixmonitor runs its own thread to monitor the channel's activity and write the mixed
audio to a file. Since this thread runs independently of the channel, it is possible that
the mixmonitor thread's channel pointer will point to freed memory when the channel either
is masqueraded or hangs up (technically, both cases are hangups, but we need to handle the
cases slightly differently).
The solution for this is to employ a datastore, which has the nice benefit of allowing us
to hook into channel masquerades and hangups and update our pointer as necessary. If this
looks familiar, this same technique is employed in app_chanspy. app_chanspy is a bit more
involved since it does a lot more operations on the channel that is being spied upon.
app_mixmonitor does have an extra touch that app_chanspy doesn't have, though. Since there
is a thread race between the channel's thread and the mixmonitor thread on a hangup, we em-
ploy a condition-and-boolean combination to ensure that the channel thread finishes with
our structure before the mixmonitor thread attempts to free it. No crashes!
(closes issue #14374)
Reported by: aragon
Patches:
14374.patch uploaded by putnopvut (license 60)
Tested by: aragon, putnopvut
........
................
Modified:
branches/1.6.0/ (props changed)
branches/1.6.0/apps/app_mixmonitor.c
Propchange: branches/1.6.0/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.
Modified: branches/1.6.0/apps/app_mixmonitor.c
URL: http://svn.digium.com/svn-view/asterisk/branches/1.6.0/apps/app_mixmonitor.c?view=diff&rev=173590&r1=173589&r2=173590
==============================================================================
--- branches/1.6.0/apps/app_mixmonitor.c (original)
+++ branches/1.6.0/apps/app_mixmonitor.c Thu Feb 5 12:39:35 2009
@@ -89,7 +89,7 @@
char *post_process;
char *name;
unsigned int flags;
- struct ast_channel *chan;
+ struct mixmonitor_ds *mixmonitor_ds;
};
enum {
@@ -115,6 +115,50 @@
AST_APP_OPTION_ARG('W', MUXFLAG_VOLUME, OPT_ARG_VOLUME),
});
+/* This structure is used as a means of making sure that our pointer to
+ * the channel we are monitoring remains valid. This is very similar to
+ * what is used in app_chanspy.c.
+ */
+struct mixmonitor_ds {
+ struct ast_channel *chan;
+ /* These condition variables are used to be sure that the channel
+ * hangup code completes before the mixmonitor thread attempts to
+ * free this structure. The combination of a bookean flag and a
+ * ast_cond_t ensure that no matter what order the threads run in,
+ * we are guaranteed to never have the waiting thread block forever
+ * in the case that the signaling thread runs first.
+ */
+ unsigned int destruction_ok;
+ ast_cond_t destruction_condition;
+ ast_mutex_t lock;
+};
+
+static void mixmonitor_ds_destroy(void *data)
+{
+ struct mixmonitor_ds *mixmonitor_ds = data;
+
+ ast_mutex_lock(&mixmonitor_ds->lock);
+ mixmonitor_ds->chan = NULL;
+ mixmonitor_ds->destruction_ok = 1;
+ ast_cond_signal(&mixmonitor_ds->destruction_condition);
+ ast_mutex_unlock(&mixmonitor_ds->lock);
+}
+
+static void mixmonitor_ds_chan_fixup(void *data, struct ast_channel *old_chan, struct ast_channel *new_chan)
+{
+ struct mixmonitor_ds *mixmonitor_ds = data;
+
+ ast_mutex_lock(&mixmonitor_ds->lock);
+ mixmonitor_ds->chan = new_chan;
+ ast_mutex_unlock(&mixmonitor_ds->lock);
+}
+
+static struct ast_datastore_info mixmonitor_ds_info = {
+ .type = "mixmonitor",
+ .destroy = mixmonitor_ds_destroy,
+ .chan_fixup = mixmonitor_ds_chan_fixup,
+};
+
static int startmon(struct ast_channel *chan, struct ast_audiohook *audiohook)
{
struct ast_channel *peer = NULL;
@@ -156,7 +200,9 @@
if (!(fr = ast_audiohook_read_frame(&mixmonitor->audiohook, SAMPLES_PER_FRAME, AST_AUDIOHOOK_DIRECTION_BOTH, AST_FORMAT_SLINEAR)))
continue;
- if (!ast_test_flag(mixmonitor, MUXFLAG_BRIDGED) || ast_bridged_channel(mixmonitor->chan)) {
+ 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))) {
+ ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
/* Initialize the file if not already done so */
if (!fs && !errflag) {
oflags = O_CREAT | O_WRONLY;
@@ -176,6 +222,8 @@
/* Write out frame */
if (fs)
ast_writestream(fs, fr);
+ } else {
+ ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
}
/* All done! free it. */
@@ -197,10 +245,44 @@
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_free(mixmonitor->mixmonitor_ds);
ast_free(mixmonitor);
-
return NULL;
+}
+
+static int setup_mixmonitor_ds(struct mixmonitor *mixmonitor, struct ast_channel *chan)
+{
+ struct ast_datastore *datastore = NULL;
+ struct mixmonitor_ds *mixmonitor_ds;
+
+ if (!(mixmonitor_ds = ast_calloc(1, sizeof(*mixmonitor_ds)))) {
+ return -1;
+ }
+
+ ast_mutex_init(&mixmonitor_ds->lock);
+ ast_cond_init(&mixmonitor_ds->destruction_condition, NULL);
+
+ if (!(datastore = ast_channel_datastore_alloc(&mixmonitor_ds_info, NULL))) {
+ ast_free(mixmonitor_ds);
+ return -1;
+ }
+
+ /* No need to lock mixmonitor_ds since this is still operating in the channel's thread */
+ mixmonitor_ds->chan = chan;
+ datastore->data = mixmonitor_ds;
+
+ ast_channel_lock(chan);
+ ast_channel_datastore_add(chan, datastore);
+ ast_channel_unlock(chan);
+
+ mixmonitor->mixmonitor_ds = mixmonitor_ds;
+ return 0;
}
static void launch_monitor_thread(struct ast_channel *chan, const char *filename, unsigned int flags,
@@ -236,7 +318,9 @@
/* Copy over flags and channel name */
mixmonitor->flags = flags;
- mixmonitor->chan = chan;
+ if (setup_mixmonitor_ds(mixmonitor, chan)) {
+ return;
+ }
mixmonitor->name = (char *) mixmonitor + sizeof(*mixmonitor);
strcpy(mixmonitor->name, chan->name);
if (!ast_strlen_zero(postprocess2)) {
@@ -269,7 +353,6 @@
}
ast_pthread_create_detached_background(&thread, NULL, mixmonitor_thread, mixmonitor);
-
}
static int mixmonitor_exec(struct ast_channel *chan, void *data)
More information about the asterisk-commits
mailing list