Skip to content
Snippets Groups Projects
Commit c23a04c7 authored by Richard Mudgett's avatar Richard Mudgett
Browse files

Better protect bridge_channel state from other threads.

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@379789 65c4cc65-6c06-0410-ace0-fbb531ad65f3
parent 7a69e6c5
Branches
Tags
No related merge requests found
...@@ -111,14 +111,12 @@ static int feature_blind_transfer(struct ast_bridge *bridge, struct ast_bridge_c ...@@ -111,14 +111,12 @@ static int feature_blind_transfer(struct ast_bridge *bridge, struct ast_bridge_c
/* Grab the extension to transfer to */ /* Grab the extension to transfer to */
if (!grab_transfer(bridge_channel->chan, exten, sizeof(exten), context)) { if (!grab_transfer(bridge_channel->chan, exten, sizeof(exten), context)) {
ast_stream_and_wait(bridge_channel->chan, "pbx-invalid", AST_DIGIT_ANY); ast_stream_and_wait(bridge_channel->chan, "pbx-invalid", AST_DIGIT_ANY);
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
return 0; return 0;
} }
/* Get a channel that is the destination we wish to call */ /* Get a channel that is the destination we wish to call */
if (!(chan = dial_transfer(bridge_channel->chan, exten, context))) { if (!(chan = dial_transfer(bridge_channel->chan, exten, context))) {
ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY); ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY);
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
return 0; return 0;
} }
...@@ -180,14 +178,12 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg ...@@ -180,14 +178,12 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg
/* Grab the extension to transfer to */ /* Grab the extension to transfer to */
if (!grab_transfer(bridge_channel->chan, exten, sizeof(exten), context)) { if (!grab_transfer(bridge_channel->chan, exten, sizeof(exten), context)) {
ast_stream_and_wait(bridge_channel->chan, "pbx-invalid", AST_DIGIT_ANY); ast_stream_and_wait(bridge_channel->chan, "pbx-invalid", AST_DIGIT_ANY);
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
return 0; return 0;
} }
/* Get a channel that is the destination we wish to call */ /* Get a channel that is the destination we wish to call */
if (!(chan = dial_transfer(bridge_channel->chan, exten, context))) { if (!(chan = dial_transfer(bridge_channel->chan, exten, context))) {
ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY); ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY);
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
return 0; return 0;
} }
...@@ -195,7 +191,6 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg ...@@ -195,7 +191,6 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg
if (!(attended_bridge = ast_bridge_new(AST_BRIDGE_CAPABILITY_1TO1MIX, 0))) { if (!(attended_bridge = ast_bridge_new(AST_BRIDGE_CAPABILITY_1TO1MIX, 0))) {
ast_hangup(chan); ast_hangup(chan);
ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY); ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY);
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
return 0; return 0;
} }
...@@ -232,7 +227,6 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg ...@@ -232,7 +227,6 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg
} }
} else { } else {
ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY); ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY);
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
} }
/* Now that all channels are out of it we can destroy the bridge and the called features structure */ /* Now that all channels are out of it we can destroy the bridge and the called features structure */
......
...@@ -118,7 +118,8 @@ int ast_bridge_technology_unregister(struct ast_bridge_technology *technology) ...@@ -118,7 +118,8 @@ int ast_bridge_technology_unregister(struct ast_bridge_technology *technology)
return current ? 0 : -1; return current ? 0 : -1;
} }
void ast_bridge_change_state(struct ast_bridge_channel *bridge_channel, enum ast_bridge_channel_state new_state) /*! \note This function assumes the bridge_channel is locked. */
static void ast_bridge_change_state_nolock(struct ast_bridge_channel *bridge_channel, enum ast_bridge_channel_state new_state)
{ {
/* Change the state on the bridge channel */ /* Change the state on the bridge channel */
bridge_channel->state = new_state; bridge_channel->state = new_state;
...@@ -126,12 +127,17 @@ void ast_bridge_change_state(struct ast_bridge_channel *bridge_channel, enum ast ...@@ -126,12 +127,17 @@ void ast_bridge_change_state(struct ast_bridge_channel *bridge_channel, enum ast
/* Only poke the channel's thread if it is not us */ /* Only poke the channel's thread if it is not us */
if (!pthread_equal(pthread_self(), bridge_channel->thread)) { if (!pthread_equal(pthread_self(), bridge_channel->thread)) {
pthread_kill(bridge_channel->thread, SIGURG); pthread_kill(bridge_channel->thread, SIGURG);
ao2_lock(bridge_channel);
ast_cond_signal(&bridge_channel->cond); ast_cond_signal(&bridge_channel->cond);
ao2_unlock(bridge_channel);
} }
} }
void ast_bridge_change_state(struct ast_bridge_channel *bridge_channel, enum ast_bridge_channel_state new_state)
{
ao2_lock(bridge_channel);
ast_bridge_change_state_nolock(bridge_channel, new_state);
ao2_unlock(bridge_channel);
}
/*! \brief Helper function to poke the bridge thread */ /*! \brief Helper function to poke the bridge thread */
static void bridge_poke(struct ast_bridge *bridge) static void bridge_poke(struct ast_bridge *bridge)
{ {
...@@ -245,15 +251,17 @@ static void bridge_force_out_all(struct ast_bridge *bridge) ...@@ -245,15 +251,17 @@ static void bridge_force_out_all(struct ast_bridge *bridge)
struct ast_bridge_channel *bridge_channel; struct ast_bridge_channel *bridge_channel;
AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) { AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) {
ao2_lock(bridge_channel);
switch (bridge_channel->state) { switch (bridge_channel->state) {
case AST_BRIDGE_CHANNEL_STATE_END: case AST_BRIDGE_CHANNEL_STATE_END:
case AST_BRIDGE_CHANNEL_STATE_HANGUP: case AST_BRIDGE_CHANNEL_STATE_HANGUP:
case AST_BRIDGE_CHANNEL_STATE_DEPART: case AST_BRIDGE_CHANNEL_STATE_DEPART:
break; break;
default: default:
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_HANGUP); ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_HANGUP);
break; break;
} }
ao2_unlock(bridge_channel);
} }
} }
...@@ -908,11 +916,6 @@ static void bridge_channel_feature(struct ast_bridge *bridge, struct ast_bridge_ ...@@ -908,11 +916,6 @@ static void bridge_channel_feature(struct ast_bridge *bridge, struct ast_bridge_
} else { } else {
ast_bridge_dtmf_stream(bridge, dtmf, bridge_channel->chan); ast_bridge_dtmf_stream(bridge, dtmf, bridge_channel->chan);
} }
/* if the channel is still in feature state, revert it back to wait state */
if (bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_FEATURE) {
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
}
} }
static void bridge_channel_talking(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel) static void bridge_channel_talking(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
...@@ -922,11 +925,10 @@ static void bridge_channel_talking(struct ast_bridge *bridge, struct ast_bridge_ ...@@ -922,11 +925,10 @@ static void bridge_channel_talking(struct ast_bridge *bridge, struct ast_bridge_
if (features && features->talker_cb) { if (features && features->talker_cb) {
features->talker_cb(bridge, bridge_channel, features->talker_pvt_data); features->talker_cb(bridge, bridge_channel, features->talker_pvt_data);
} }
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
} }
/*! \brief Internal function that plays back DTMF on a bridge channel */ /*! \brief Internal function that plays back DTMF on a bridge channel */
static void bridge_channel_dtmf_stream(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel) static void bridge_channel_dtmf_stream(struct ast_bridge_channel *bridge_channel)
{ {
char dtmf_q[8] = ""; char dtmf_q[8] = "";
...@@ -935,8 +937,6 @@ static void bridge_channel_dtmf_stream(struct ast_bridge *bridge, struct ast_bri ...@@ -935,8 +937,6 @@ static void bridge_channel_dtmf_stream(struct ast_bridge *bridge, struct ast_bri
ast_debug(1, "Playing DTMF stream '%s' out to bridge channel %p\n", dtmf_q, bridge_channel); ast_debug(1, "Playing DTMF stream '%s' out to bridge channel %p\n", dtmf_q, bridge_channel);
ast_dtmf_stream(bridge_channel->chan, NULL, dtmf_q, 250, 0); ast_dtmf_stream(bridge_channel->chan, NULL, dtmf_q, 250, 0);
ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
} }
/*! \brief Join a channel to a bridge and handle anything the bridge may want us to do */ /*! \brief Join a channel to a bridge and handle anything the bridge may want us to do */
...@@ -1021,11 +1021,23 @@ static enum ast_bridge_channel_state bridge_channel_join(struct ast_bridge_chann ...@@ -1021,11 +1021,23 @@ static enum ast_bridge_channel_state bridge_channel_join(struct ast_bridge_chann
ao2_unlock(bridge_channel->bridge); ao2_unlock(bridge_channel->bridge);
bridge_channel_feature(bridge_channel->bridge, bridge_channel); bridge_channel_feature(bridge_channel->bridge, bridge_channel);
ao2_lock(bridge_channel->bridge); ao2_lock(bridge_channel->bridge);
ao2_lock(bridge_channel);
if (bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_FEATURE) {
ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
}
ao2_unlock(bridge_channel);
bridge_channel_unsuspend(bridge_channel->bridge, bridge_channel); bridge_channel_unsuspend(bridge_channel->bridge, bridge_channel);
break; break;
case AST_BRIDGE_CHANNEL_STATE_DTMF: case AST_BRIDGE_CHANNEL_STATE_DTMF:
bridge_channel_suspend(bridge_channel->bridge, bridge_channel); bridge_channel_suspend(bridge_channel->bridge, bridge_channel);
bridge_channel_dtmf_stream(bridge_channel->bridge, bridge_channel); ao2_unlock(bridge_channel->bridge);
bridge_channel_dtmf_stream(bridge_channel);
ao2_lock(bridge_channel->bridge);
ao2_lock(bridge_channel);
if (bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_DTMF) {
ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
}
ao2_unlock(bridge_channel);
bridge_channel_unsuspend(bridge_channel->bridge, bridge_channel); bridge_channel_unsuspend(bridge_channel->bridge, bridge_channel);
break; break;
case AST_BRIDGE_CHANNEL_STATE_START_TALKING: case AST_BRIDGE_CHANNEL_STATE_START_TALKING:
...@@ -1033,6 +1045,16 @@ static enum ast_bridge_channel_state bridge_channel_join(struct ast_bridge_chann ...@@ -1033,6 +1045,16 @@ static enum ast_bridge_channel_state bridge_channel_join(struct ast_bridge_chann
ao2_unlock(bridge_channel->bridge); ao2_unlock(bridge_channel->bridge);
bridge_channel_talking(bridge_channel->bridge, bridge_channel); bridge_channel_talking(bridge_channel->bridge, bridge_channel);
ao2_lock(bridge_channel->bridge); ao2_lock(bridge_channel->bridge);
ao2_lock(bridge_channel);
switch (bridge_channel->state) {
case AST_BRIDGE_CHANNEL_STATE_START_TALKING:
case AST_BRIDGE_CHANNEL_STATE_STOP_TALKING:
ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
break;
default:
break;
}
ao2_unlock(bridge_channel);
break; break;
default: default:
break; break;
...@@ -1141,11 +1163,9 @@ enum ast_bridge_channel_state ast_bridge_join(struct ast_bridge *bridge, ...@@ -1141,11 +1163,9 @@ enum ast_bridge_channel_state ast_bridge_join(struct ast_bridge *bridge,
state = bridge_channel_join(bridge_channel); state = bridge_channel_join(bridge_channel);
/* Cleanup all the data in the bridge channel after it leaves the bridge. */ /* Cleanup all the data in the bridge channel after it leaves the bridge. */
ao2_lock(bridge_channel);
bridge_channel->chan = NULL; bridge_channel->chan = NULL;
bridge_channel->swap = NULL; bridge_channel->swap = NULL;
bridge_channel->features = NULL; bridge_channel->features = NULL;
ao2_unlock(bridge_channel);
ao2_ref(bridge_channel, -1); ao2_ref(bridge_channel, -1);
return state; return state;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment