[asterisk-commits] mmichelson: branch 1.6.0 r166278 - in /branches/1.6.0: include/asterisk/ main/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Dec 22 10:30:26 CST 2008
Author: mmichelson
Date: Mon Dec 22 10:30:25 2008
New Revision: 166278
URL: http://svn.digium.com/view/asterisk?view=rev&rev=166278
Log:
When merging the fix for issue #14118, I found that
the issue didn't affect 1.6.0, but in this case that's
not an especially good thing, because it means that
the fix for issue #13496 was not merged into 1.6.0 in
the first place. This commit kills two birds with one
stone by putting both fixes in the 1.6.0 branch
Modified:
branches/1.6.0/include/asterisk/file.h
branches/1.6.0/include/asterisk/frame.h
branches/1.6.0/main/file.c
branches/1.6.0/main/frame.c
Modified: branches/1.6.0/include/asterisk/file.h
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/include/asterisk/file.h?view=diff&rev=166278&r1=166277&r2=166278
==============================================================================
--- branches/1.6.0/include/asterisk/file.h (original)
+++ branches/1.6.0/include/asterisk/file.h Mon Dec 22 10:30:25 2008
@@ -315,6 +315,21 @@
*/
struct ast_frame *ast_readframe(struct ast_filestream *s);
+/*!\brief destroy a filestream using an ast_frame as input
+ *
+ * This is a hack that is used also by the ast_trans_pvt and
+ * ast_dsp structures. When a structure contains an ast_frame
+ * pointer as one of its fields. It may be that the frame is
+ * still used after the outer structure is freed. This leads to
+ * invalid memory accesses. This function allows for us to hold
+ * off on destroying the ast_filestream until we are done using
+ * the ast_frame pointer that is part of it
+ *
+ * \param fr The ast_frame that is part of an ast_filestream we wish
+ * to free.
+ */
+void ast_filestream_frame_freed(struct ast_frame *fr);
+
#if defined(__cplusplus) || defined(c_plusplus)
}
#endif
Modified: branches/1.6.0/include/asterisk/frame.h
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/include/asterisk/frame.h?view=diff&rev=166278&r1=166277&r2=166278
==============================================================================
--- branches/1.6.0/include/asterisk/frame.h (original)
+++ branches/1.6.0/include/asterisk/frame.h Mon Dec 22 10:30:25 2008
@@ -134,6 +134,10 @@
* The dsp cannot be free'd if the frame inside of it still has
* this flag set. */
AST_FRFLAG_FROM_DSP = (1 << 2),
+ /*! This frame came from a filestream and is still the original frame.
+ * The filestream cannot be free'd if the frame inside of it still has
+ * this flag set. */
+ AST_FRFLAG_FROM_FILESTREAM = (1 << 3),
};
/*! \brief Data structure associated with a single frame of data
Modified: branches/1.6.0/main/file.c
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/main/file.c?view=diff&rev=166278&r1=166277&r2=166278
==============================================================================
--- branches/1.6.0/main/file.c (original)
+++ branches/1.6.0/main/file.c Mon Dec 22 10:30:25 2008
@@ -43,6 +43,7 @@
#include "asterisk/pbx.h"
#include "asterisk/linkedlists.h"
#include "asterisk/module.h"
+#include "asterisk/astobj2.h"
/*
* The following variable controls the layout of localized sound files.
@@ -279,12 +280,57 @@
return 0;
}
+static void filestream_destructor(void *arg)
+{
+ char *cmd = NULL;
+ size_t size = 0;
+ struct ast_filestream *f = arg;
+
+ /* Stop a running stream if there is one */
+ if (f->owner) {
+ if (f->fmt->format < AST_FORMAT_AUDIO_MASK) {
+ f->owner->stream = NULL;
+ AST_SCHED_DEL(f->owner->sched, f->owner->streamid);
+#ifdef HAVE_DAHDI
+ ast_settimeout(f->owner, 0, NULL, NULL);
+#endif
+ } else {
+ f->owner->vstream = NULL;
+ AST_SCHED_DEL(f->owner->sched, f->owner->vstreamid);
+ }
+ }
+ /* destroy the translator on exit */
+ if (f->trans)
+ ast_translator_free_path(f->trans);
+
+ if (f->realfilename && f->filename) {
+ size = strlen(f->filename) + strlen(f->realfilename) + 15;
+ cmd = alloca(size);
+ memset(cmd,0,size);
+ snprintf(cmd,size,"/bin/mv -f %s %s",f->filename,f->realfilename);
+ ast_safe_system(cmd);
+ }
+
+ if (f->filename)
+ free(f->filename);
+ if (f->realfilename)
+ free(f->realfilename);
+ if (f->fmt->close)
+ f->fmt->close(f);
+ fclose(f->f);
+ if (f->vfs)
+ ast_closestream(f->vfs);
+ if (f->orig_chan_name)
+ free((void *) f->orig_chan_name);
+ ast_module_unref(f->fmt->module);
+}
+
static struct ast_filestream *get_filestream(struct ast_format *fmt, FILE *bfile)
{
struct ast_filestream *s;
int l = sizeof(*s) + fmt->buf_size + fmt->desc_size; /* total allocation size */
- if ( (s = ast_calloc(1, l)) == NULL)
+ if ( (s = ao2_alloc(l, filestream_destructor)) == NULL)
return NULL;
s->fmt = fmt;
s->f = bfile;
@@ -637,6 +683,10 @@
int whennext = 0;
if (s && s->fmt)
f = s->fmt->read(s, &whennext);
+ if (f) {
+ ast_set_flag(f, AST_FRFLAG_FROM_FILESTREAM);
+ ao2_ref(s, +1);
+ }
return f;
}
@@ -659,6 +709,10 @@
goto return_failure;
fr = s->fmt->read(s, &whennext);
+ if (fr) {
+ ast_set_flag(fr, AST_FRFLAG_FROM_FILESTREAM);
+ ao2_ref(s, +1);
+ }
if (!fr /* stream complete */ || ast_write(s->owner, fr) /* error writing */) {
if (fr)
ast_log(LOG_WARNING, "Failed to write frame\n");
@@ -715,6 +769,10 @@
while (!whennext) {
struct ast_frame *fr = s->fmt->read(s, &whennext);
+ if (fr) {
+ ast_set_flag(fr, AST_FRFLAG_FROM_FILESTREAM);
+ ao2_ref(s, +1);
+ }
if (!fr || ast_write(s->owner, fr)) { /* no stream or error, as above */
if (fr)
ast_log(LOG_WARNING, "Failed to write frame\n");
@@ -792,46 +850,21 @@
int ast_closestream(struct ast_filestream *f)
{
- char *cmd = NULL;
- size_t size = 0;
- /* Stop a running stream if there is one */
- if (f->owner) {
- if (f->fmt->format & AST_FORMAT_AUDIO_MASK) {
- f->owner->stream = NULL;
- AST_SCHED_DEL(f->owner->sched, f->owner->streamid);
-#ifdef HAVE_DAHDI
- ast_settimeout(f->owner, 0, NULL, NULL);
-#endif
- } else {
- f->owner->vstream = NULL;
- AST_SCHED_DEL(f->owner->sched, f->owner->vstreamid);
- }
- }
- /* destroy the translator on exit */
- if (f->trans)
- ast_translator_free_path(f->trans);
-
- if (f->realfilename && f->filename) {
- size = strlen(f->filename) + strlen(f->realfilename) + 15;
- cmd = alloca(size);
- memset(cmd, 0, size);
- snprintf(cmd, size, "/bin/mv -f %s %s", f->filename, f->realfilename);
- ast_safe_system(cmd);
- }
-
- if (f->filename)
- ast_free(f->filename);
- if (f->realfilename)
- ast_free(f->realfilename);
- if (f->fmt->close)
- f->fmt->close(f);
- fclose(f->f);
- if (f->vfs)
- ast_closestream(f->vfs);
- if (f->orig_chan_name)
- free((void *) f->orig_chan_name);
- ast_module_unref(f->fmt->module);
- ast_free(f);
+ if (ast_test_flag(&f->fr, AST_FRFLAG_FROM_FILESTREAM)) {
+ /* If this flag is still set, it essentially means that the reference
+ * count of f is non-zero. We can't destroy this filestream until
+ * whatever is using the filestream's frame has finished.
+ *
+ * Since this was called, however, we need to remove the reference from
+ * when this filestream was first allocated. That way, when the embedded
+ * frame is freed, the refcount will reach 0 and we can finish destroying
+ * this filestream properly.
+ */
+ ao2_ref(f, -1);
+ return 0;
+ }
+
+ ao2_ref(f, -1);
return 0;
}
@@ -1237,6 +1270,17 @@
-1, -1, context);
}
+void ast_filestream_frame_freed(struct ast_frame *fr)
+{
+ struct ast_filestream *fs;
+
+ ast_clear_flag(fr, AST_FRFLAG_FROM_FILESTREAM);
+
+ fs = (struct ast_filestream *) (((char *) fr) - offsetof(struct ast_filestream, fr));
+
+ ao2_ref(fs, -1);
+}
+
/*
* if the file name is non-empty, try to play it.
* Return 0 if success, -1 if error, digit if interrupted by a digit.
Modified: branches/1.6.0/main/frame.c
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/main/frame.c?view=diff&rev=166278&r1=166277&r2=166278
==============================================================================
--- branches/1.6.0/main/frame.c (original)
+++ branches/1.6.0/main/frame.c Mon Dec 22 10:30:25 2008
@@ -38,6 +38,7 @@
#include "asterisk/linkedlists.h"
#include "asterisk/translate.h"
#include "asterisk/dsp.h"
+#include "asterisk/file.h"
#ifdef TRACE_FRAMES
static int headers;
@@ -307,10 +308,13 @@
void ast_frame_free(struct ast_frame *fr, int cache)
{
- if (ast_test_flag(fr, AST_FRFLAG_FROM_TRANSLATOR))
+ if (ast_test_flag(fr, AST_FRFLAG_FROM_TRANSLATOR)) {
ast_translate_frame_freed(fr);
- else if (ast_test_flag(fr, AST_FRFLAG_FROM_DSP))
+ } else if (ast_test_flag(fr, AST_FRFLAG_FROM_DSP)) {
ast_dsp_frame_freed(fr);
+ } else if (ast_test_flag(fr, AST_FRFLAG_FROM_FILESTREAM)) {
+ ast_filestream_frame_freed(fr);
+ }
if (!fr->mallocd)
return;
More information about the asterisk-commits
mailing list