diff options
author | Richard Mudgett <rmudgett@digium.com> | 2011-09-02 21:09:31 +0000 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2011-09-02 21:09:31 +0000 |
commit | 35e27201c7b942396e01fcabf4aeba2edfe9f544 (patch) | |
tree | 652f71093d337841ac2c6ac63274b815ec402f2a /res/res_musiconhold.c | |
parent | 220bf145570d6ad584c174fc531793d699976400 (diff) |
Merged revisions 334357 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/10
................
r334357 | rmudgett | 2011-09-02 16:08:16 -0500 (Fri, 02 Sep 2011) | 26 lines
Merged revisions 334355 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8
........
r334355 | rmudgett | 2011-09-02 15:59:49 -0500 (Fri, 02 Sep 2011) | 19 lines
MusicOnHold has extra unref which may lead to memory corruption and crash.
The problem happens when a call is disconnected and you had started a MOH
class that does not use the files mode. If you define REF_DEBUG and
recreate the problem, it will announce itself with the following warning:
Attempt to unref mohclass 0xb70722e0 (default) when only 1 ref remained,
and class is still in a container!
* Fixed moh_alloc() and moh_release() functions not handling the
state->class reference consistently.
(closes issue ASTERISK-18346)
Reported by: Mark Murawski
Patches:
jira_asterisk_18346_v1.8.patch (license #5621) patch uploaded by rmudgett
Tested by: rmudgett, Mark Murawski
Review: https://reviewboard.asterisk.org/r/1404/
........
................
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@334358 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'res/res_musiconhold.c')
-rw-r--r-- | res/res_musiconhold.c | 39 |
1 files changed, 29 insertions, 10 deletions
diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c index c2207d6ce..bc3d2f343 100644 --- a/res/res_musiconhold.c +++ b/res/res_musiconhold.c @@ -151,6 +151,7 @@ static const char stop_moh[] = "StopMusicOnHold"; static int respawn_time = 20; struct moh_files_state { + /*! Holds a reference to the MOH class. */ struct mohclass *class; char name[MAX_MUSICCLASS]; struct ast_format origwfmt; @@ -232,7 +233,7 @@ static struct mohclass *_mohclass_unref(struct mohclass *class, const char *tag, { struct mohclass *dup; if ((dup = ao2_find(mohclasses, class, OBJ_POINTER))) { - if (_ao2_ref_debug(dup, -1, (char *) tag, (char *) file, line, funcname) == 2) { + if (__ao2_ref_debug(dup, -1, (char *) tag, (char *) file, line, funcname) == 2) { FILE *ref = fopen("/tmp/refs", "a"); if (ref) { fprintf(ref, "%p =1 %s:%d:%s (%s) BAD ATTEMPT!\n", class, file, line, funcname, tag); @@ -433,10 +434,13 @@ static void *moh_files_alloc(struct ast_channel *chan, void *params) ast_module_ref(ast_module_info->self); } else { state = chan->music_state; - } - - if (!state) { - return NULL; + if (!state) { + return NULL; + } + if (state->class) { + mohclass_unref(state->class, "Uh Oh. Restarting MOH with an active class"); + ast_log(LOG_WARNING, "Uh Oh. Restarting MOH with an active class\n"); + } } /* LOGIC: Comparing an unrefcounted pointer is a really bad idea, because @@ -936,6 +940,12 @@ static void moh_release(struct ast_channel *chan, void *data) ast_free(moh); if (chan) { + struct moh_files_state *state; + + state = chan->music_state; + if (state && state->class) { + state->class = mohclass_unref(state->class, "Unreffing channel's music class upon deactivation of generator"); + } if (oldwfmt.id && ast_set_write_format(chan, &oldwfmt)) { ast_log(LOG_WARNING, "Unable to restore channel '%s' to format %s\n", chan->name, ast_getformatname(&oldwfmt)); @@ -954,13 +964,17 @@ static void *moh_alloc(struct ast_channel *chan, void *params) /* Initiating music_state for current channel. Channel should know name of moh class */ if (!chan->music_state && (state = ast_calloc(1, sizeof(*state)))) { chan->music_state = state; - state->class = mohclass_ref(class, "Copying reference into state container"); ast_module_ref(ast_module_info->self); - } else + } else { state = chan->music_state; - if (state && state->class != class) { + if (!state) { + return NULL; + } + if (state->class) { + mohclass_unref(state->class, "Uh Oh. Restarting MOH with an active class"); + ast_log(LOG_WARNING, "Uh Oh. Restarting MOH with an active class\n"); + } memset(state, 0, sizeof(*state)); - state->class = class; } if ((res = mohalloc(class))) { @@ -969,6 +983,8 @@ static void *moh_alloc(struct ast_channel *chan, void *params) ast_log(LOG_WARNING, "Unable to set channel '%s' to format '%s'\n", chan->name, ast_codec2str(&class->format)); moh_release(NULL, res); res = NULL; + } else { + state->class = mohclass_ref(class, "Placing reference into state container"); } ast_verb(3, "Started music on hold, class '%s', on channel '%s'\n", class->name, chan->name); } @@ -1285,7 +1301,10 @@ static void local_ast_moh_cleanup(struct ast_channel *chan) if (state) { if (state->class) { - state->class = mohclass_unref(state->class, "Channel MOH state destruction"); + /* This should never happen. We likely just leaked some resource. */ + state->class = + mohclass_unref(state->class, "Uh Oh. Cleaning up MOH with an active class"); + ast_log(LOG_WARNING, "Uh Oh. Cleaning up MOH with an active class\n"); } ast_free(chan->music_state); chan->music_state = NULL; |