sysmocom
/
asterisk
11
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

res_stasis: Fix stale data in ARI bridges 

Fixed an issue that resulted in "Allocation failed" each time an ARI
request was made to start playing MOH on a bridge.

In bridge_moh_create() we were attaching the after bridge callbacks to
chan which is the ;1 channel of the unreal channel pair.  We should have
attached them to the ;2 channel which is pushed into the bridge by
ast_unreal_channel_push_to_bridge().  The callbacks are called when the
specific channel leaves the bridging system.  Since the ;1 channel is
never put into a bridge the callbacks never get called.  The callbacks
then never remove the moh_wrapper from the app_bridges_moh container.  As
a result we cannot find the channel associated with the wrapper to start
MOH because it has hungup.  This is the reason causing the reported issue.

* Rather than using after bridge callbacks to cleanup, we now have
moh_channel_thread() doing the cleanup when the channel hangs up.

* Fixed moh_channel_thread() accumulating control frames on the stasis
bridge MOH channel until MOH is stopped.  Control frames are no longer
accumulated while MOH is playing.

* Fixed channel ref counting issue.  stasis_app_bridge_moh_channel() may
or may not return a channel ref.  As a result ast_ari_bridges_start_moh()
wouldn't know it may have a channel ref to release.
stasis_app_bridge_moh_channel() will now return a ref with the channel it
returns.

* Eliminated RAII_VAR in bridge_moh_create().

ASTERISK-26094 #close

Change-Id: Ibff479e167b3320c68aaabfada7e1d0ef7bd548c
This commit is contained in:
Moritz Fain 2018-06-26 16:17:37 +02:00 committed by Richard Mudgett
parent 94ef074cc7
commit 1f68614f3c
2 changed files with 37 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
res/ari/resource_bridges.c
View File

@ -825,6 +825,7 @@ void ast_ari_bridges_start_moh(struct ast_variable *headers,
}
ast_moh_start(moh_channel, moh_class, NULL);
ast_channel_cleanup(moh_channel);
ast_ari_response_no_content(response);

81
res/res_stasis.c
View File

@ -472,29 +472,6 @@ static int bridges_channel_sort_fn(const void *obj_left, const void *obj_right,
return cmp;
}
/*! Removes the bridge to music on hold channel link */
static void remove_bridge_moh(char *bridge_id)
{
ao2_find(app_bridges_moh, bridge_id, OBJ_SEARCH_KEY | OBJ_UNLINK | OBJ_NODATA);
ast_free(bridge_id);
}
/*! After bridge failure callback for moh channels */
static void moh_after_bridge_cb_failed(enum ast_bridge_after_cb_reason reason, void *data)
{
char *bridge_id = data;
remove_bridge_moh(bridge_id);
}
/*! After bridge callback for moh channels */
static void moh_after_bridge_cb(struct ast_channel *chan, void *data)
{
char *bridge_id = data;
remove_bridge_moh(bridge_id);
}
/*! Request a bridge MOH channel */
static struct ast_channel *prepare_bridge_moh_channel(void)
{
@ -517,11 +494,34 @@ static struct ast_channel *prepare_bridge_moh_channel(void)
/*! Provides the moh channel with a thread so it can actually play its music */
static void *moh_channel_thread(void *data)
{
struct ast_channel *moh_channel = data;
struct stasis_app_bridge_channel_wrapper *moh_wrapper = data;
struct ast_channel *moh_channel = ast_channel_get_by_name(moh_wrapper->channel_id);
struct ast_frame *f;
while (!ast_safe_sleep(moh_channel, 1000)) {
if (!moh_channel) {
ao2_unlink(app_bridges_moh, moh_wrapper);
ao2_ref(moh_wrapper, -1);
return NULL;
}
/* Read and discard any frame coming from the stasis bridge. */
for (;;) {
if (ast_waitfor(moh_channel, -1) < 0) {
/* Error or hungup */
break;
}
f = ast_read(moh_channel);
if (!f) {
/* Hungup */
break;
}
ast_frfree(f);
}
ao2_unlink(app_bridges_moh, moh_wrapper);
ao2_ref(moh_wrapper, -1);
ast_moh_stop(moh_channel);
ast_hangup(moh_channel);
@ -539,15 +539,10 @@ static void *moh_channel_thread(void *data)
*/
static struct ast_channel *bridge_moh_create(struct ast_bridge *bridge)
{
RAII_VAR(struct stasis_app_bridge_channel_wrapper *, new_wrapper, NULL, ao2_cleanup);
RAII_VAR(char *, bridge_id, ast_strdup(bridge->uniqueid), ast_free);
struct stasis_app_bridge_channel_wrapper *new_wrapper;
struct ast_channel *chan;
pthread_t threadid;
if (!bridge_id) {
return NULL;
}
chan = prepare_bridge_moh_channel();
if (!chan) {
return NULL;
@ -558,14 +553,6 @@ static struct ast_channel *bridge_moh_create(struct ast_bridge *bridge)
return NULL;
}
/* The after bridge callback assumes responsibility of the bridge_id. */
if (ast_bridge_set_after_callback(chan,
moh_after_bridge_cb, moh_after_bridge_cb_failed, bridge_id)) {
ast_hangup(chan);
return NULL;
}
bridge_id = NULL;
if (ast_unreal_channel_push_to_bridge(chan, bridge,
AST_BRIDGE_CHANNEL_FLAG_IMMOVABLE | AST_BRIDGE_CHANNEL_FLAG_LONELY)) {
ast_hangup(chan);
@ -579,21 +566,25 @@ static struct ast_channel *bridge_moh_create(struct ast_bridge *bridge)
return NULL;
}
if (ast_string_field_init(new_wrapper, 32)) {
if (ast_string_field_init(new_wrapper, AST_UUID_STR_LEN + AST_CHANNEL_NAME)
|| ast_string_field_set(new_wrapper, bridge_id, bridge->uniqueid)
|| ast_string_field_set(new_wrapper, channel_id, ast_channel_uniqueid(chan))) {
ao2_ref(new_wrapper, -1);
ast_hangup(chan);
return NULL;
}
ast_string_field_set(new_wrapper, bridge_id, bridge->uniqueid);
ast_string_field_set(new_wrapper, channel_id, ast_channel_uniqueid(chan));
if (!ao2_link_flags(app_bridges_moh, new_wrapper, OBJ_NOLOCK)) {
ao2_ref(new_wrapper, -1);
ast_hangup(chan);
return NULL;
}
if (ast_pthread_create_detached(&threadid, NULL, moh_channel_thread, chan)) {
/* Pass the new_wrapper ref to moh_channel_thread() */
if (ast_pthread_create_detached(&threadid, NULL, moh_channel_thread, new_wrapper)) {
ast_log(LOG_ERROR, "Failed to create channel thread. Abandoning MOH channel creation.\n");
ao2_unlink_flags(app_bridges_moh, new_wrapper, OBJ_NOLOCK);
ao2_ref(new_wrapper, -1);
ast_hangup(chan);
return NULL;
}
@ -2180,8 +2171,8 @@ static int load_module(void)
app_controls = ao2_container_alloc(CONTROLS_NUM_BUCKETS, control_hash, control_compare);
app_bridges = ao2_container_alloc(BRIDGES_NUM_BUCKETS, bridges_hash, bridges_compare);
app_bridges_moh = ao2_container_alloc_hash(
AO2_ALLOC_OPT_LOCK_MUTEX, AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT,
37, bridges_channel_hash_fn, bridges_channel_sort_fn, NULL);
AO2_ALLOC_OPT_LOCK_MUTEX, 0,
37, bridges_channel_hash_fn, NULL, NULL);
app_bridges_playback = ao2_container_alloc_hash(
AO2_ALLOC_OPT_LOCK_MUTEX, AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT,
37, bridges_channel_hash_fn, bridges_channel_sort_fn, NULL);