moh: fix a refcount error with realtime MOH
I observed a crash in res_musiconhold on an Asterisk 11 system using realtime MOH. Investigation of the backtrace showed a corrupt mohclass, implying that it got destroyed before the code expected it to. I went looking for reference counting errors that could have caused this crash and this patch this result. It contains 2 changes. 1) Remove a usless block of code that was impossible to reach. There was even a comment indicating that it was impossible to reach. The conditional includes "!ast_test_flag(global_flags, MOH_CACHERTCLASSES)" and it's inside of an if block with the opposite check "ast_test_flag(global_flags, MOH_CACHERTCLASSES)". There's no good reason to keep it around. 2) A similar block to #1 contained a reference counting error. It stores state->class in the local variable mohclass without increasing its reference count. The reference count on mohclass is decremented at the end of the function. This block of code probably very rarely runs, which would help explain why this system was working fine for many months before experiencing a crash. Review: https://reviewboard.asterisk.org/r/3282/ ........ Merged revisions 410043 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 410044 from http://svn.asterisk.org/svn/asterisk/branches/11 ........ Merged revisions 410090 from http://svn.asterisk.org/svn/asterisk/branches/12 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@410091 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
parent
a4906e9f86
commit
fbf8700c10
1 changed files with 1 additions and 7 deletions
|
@ -1486,12 +1486,6 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
|
||||||
if (state && state->class) {
|
if (state && state->class) {
|
||||||
/* Class already exist for this channel */
|
/* Class already exist for this channel */
|
||||||
ast_log(LOG_NOTICE, "This channel already has a MOH class attached (%s)!\n", state->class->name);
|
ast_log(LOG_NOTICE, "This channel already has a MOH class attached (%s)!\n", state->class->name);
|
||||||
if (state->class->realtime && !ast_test_flag(global_flags, MOH_CACHERTCLASSES) && !strcasecmp(mohclass->name, state->class->name)) {
|
|
||||||
/* we found RT class with the same name, seems like we should continue playing existing one */
|
|
||||||
/* XXX This code is impossible to reach */
|
|
||||||
mohclass = mohclass_unref(mohclass, "unreffing potential mohclass (channel already has a class)");
|
|
||||||
mohclass = state->class;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
/* We don't want moh_register to unref the mohclass because we do it at the end of this function as well.
|
/* We don't want moh_register to unref the mohclass because we do it at the end of this function as well.
|
||||||
* If we allowed moh_register to unref the mohclass,too, then the count would be off by one. The result would
|
* If we allowed moh_register to unref the mohclass,too, then the count would be off by one. The result would
|
||||||
|
@ -1544,7 +1538,7 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
|
||||||
if (state->class->realtime && !ast_test_flag(global_flags, MOH_CACHERTCLASSES) && !strcasecmp(mohclass->name, state->class->name)) {
|
if (state->class->realtime && !ast_test_flag(global_flags, MOH_CACHERTCLASSES) && !strcasecmp(mohclass->name, state->class->name)) {
|
||||||
/* we found RT class with the same name, seems like we should continue playing existing one */
|
/* we found RT class with the same name, seems like we should continue playing existing one */
|
||||||
mohclass = mohclass_unref(mohclass, "unreffing potential mohclass (channel already has one)");
|
mohclass = mohclass_unref(mohclass, "unreffing potential mohclass (channel already has one)");
|
||||||
mohclass = state->class;
|
mohclass = mohclass_ref(state->class, "using existing class from state");
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
if (ast_pthread_create_background(&mohclass->thread, NULL, monmp3thread, mohclass)) {
|
if (ast_pthread_create_background(&mohclass->thread, NULL, monmp3thread, mohclass)) {
|
||||||
|
|
Loading…
Reference in a new issue