Merge "bridge.c: Fixed race condition during attended transfer" into 13

changes/07/807/8
Mark Michelson 10 years ago committed by Gerrit Code Review
commit ca65ddcd19

@ -509,6 +509,8 @@ enum ast_bridge_impart_flags {
* \param features Bridge features structure.
* \param flags defined by enum ast_bridge_impart_flags.
*
* \note The given bridge must be unlocked when calling this function.
*
* \note The features parameter must be NULL or obtained by
* ast_bridge_features_new(). You must not dereference features
* after calling even if the call fails.

@ -129,11 +129,48 @@ int bridge_channel_internal_push(struct ast_bridge_channel *bridge_channel);
*/
void bridge_channel_internal_pull(struct ast_bridge_channel *bridge_channel);
/*!
* \brief Internal bridge channel wait condition and associated result.
*/
struct bridge_channel_internal_cond {
/*! Lock for the data structure */
ast_mutex_t lock;
/*! Wait condition */
ast_cond_t cond;
/*! Wait until done */
int done;
/*! The bridge channel */
struct ast_bridge_channel *bridge_channel;
};
/*!
* \internal
* \brief Wait for the expected signal.
* \since 13.5.0
*
* \param cond the wait object
*
* \return Nothing
*/
void bridge_channel_internal_wait(struct bridge_channel_internal_cond *cond);
/*!
* \internal
* \brief Signal the condition wait.
* \since 13.5.0
*
* \param cond the wait object
*
* \return Nothing
*/
void bridge_channel_internal_signal(struct bridge_channel_internal_cond *cond);
/*!
* \internal
* \brief Join the bridge_channel to the bridge (blocking)
*
* \param bridge_channel The Channel in the bridge
* \param cond data used for signaling
*
* \note The bridge_channel->swap holds a channel reference for the swap
* channel going into the bridging system. The ref ensures that the swap
@ -148,7 +185,8 @@ void bridge_channel_internal_pull(struct ast_bridge_channel *bridge_channel);
* \retval 0 bridge channel successfully joined the bridge
* \retval -1 bridge channel failed to join the bridge
*/
int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel);
int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel,
struct bridge_channel_internal_cond *cond);
/*!
* \internal

@ -1551,7 +1551,7 @@ int ast_bridge_join(struct ast_bridge *bridge,
}
if (!res) {
res = bridge_channel_internal_join(bridge_channel);
res = bridge_channel_internal_join(bridge_channel, NULL);
}
/* Cleanup all the data in the bridge channel after it leaves the bridge. */
@ -1581,13 +1581,14 @@ join_exit:;
/*! \brief Thread responsible for imparted bridged channels to be departed */
static void *bridge_channel_depart_thread(void *data)
{
struct ast_bridge_channel *bridge_channel = data;
struct bridge_channel_internal_cond *cond = data;
struct ast_bridge_channel *bridge_channel = cond->bridge_channel;
if (bridge_channel->callid) {
ast_callid_threadassoc_add(bridge_channel->callid);
}
bridge_channel_internal_join(bridge_channel);
bridge_channel_internal_join(bridge_channel, cond);
/*
* cleanup
@ -1608,14 +1609,15 @@ static void *bridge_channel_depart_thread(void *data)
/*! \brief Thread responsible for independent imparted bridged channels */
static void *bridge_channel_ind_thread(void *data)
{
struct ast_bridge_channel *bridge_channel = data;
struct bridge_channel_internal_cond *cond = data;
struct ast_bridge_channel *bridge_channel = cond->bridge_channel;
struct ast_channel *chan;
if (bridge_channel->callid) {
ast_callid_threadassoc_add(bridge_channel->callid);
}
bridge_channel_internal_join(bridge_channel);
bridge_channel_internal_join(bridge_channel, cond);
chan = bridge_channel->chan;
/* cleanup */
@ -1699,13 +1701,27 @@ int ast_bridge_impart(struct ast_bridge *bridge,
/* Actually create the thread that will handle the channel */
if (!res) {
struct bridge_channel_internal_cond cond = {
.done = 0,
.bridge_channel = bridge_channel
};
ast_mutex_init(&cond.lock);
ast_cond_init(&cond.cond, NULL);
if ((flags & AST_BRIDGE_IMPART_CHAN_MASK) == AST_BRIDGE_IMPART_CHAN_INDEPENDENT) {
res = ast_pthread_create_detached(&bridge_channel->thread, NULL,
bridge_channel_ind_thread, bridge_channel);
bridge_channel_ind_thread, &cond);
} else {
res = ast_pthread_create(&bridge_channel->thread, NULL,
bridge_channel_depart_thread, bridge_channel);
bridge_channel_depart_thread, &cond);
}
if (!res) {
bridge_channel_internal_wait(&cond);
}
ast_cond_destroy(&cond.cond);
ast_mutex_destroy(&cond.lock);
}
if (res) {
@ -3955,6 +3971,15 @@ static enum ast_transfer_result attended_transfer_bridge(struct ast_channel *cha
struct ast_channel *chan2, struct ast_bridge *bridge1, struct ast_bridge *bridge2,
struct ast_attended_transfer_message *transfer_msg)
{
#define BRIDGE_LOCK_ONE_OR_BOTH(b1, b2) \
do { \
if (b2) { \
ast_bridge_lock_both(b1, b2); \
} else { \
ast_bridge_lock(b1); \
} \
} while (0)
static const char *dest = "_attended@transfer/m";
struct ast_channel *local_chan;
int cause;
@ -3985,8 +4010,18 @@ static enum ast_transfer_result attended_transfer_bridge(struct ast_channel *cha
return AST_BRIDGE_TRANSFER_FAIL;
}
/*
* Since bridges need to be unlocked before entering ast_bridge_impart and
* core_local may call into it then the bridges need to be unlocked here.
*/
ast_bridge_unlock(bridge1);
if (bridge2) {
ast_bridge_unlock(bridge2);
}
if (ast_call(local_chan, dest, 0)) {
ast_hangup(local_chan);
BRIDGE_LOCK_ONE_OR_BOTH(bridge1, bridge2);
return AST_BRIDGE_TRANSFER_FAIL;
}
@ -3996,8 +4031,10 @@ static enum ast_transfer_result attended_transfer_bridge(struct ast_channel *cha
AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
ast_hangup(local_chan);
ao2_cleanup(local_chan);
BRIDGE_LOCK_ONE_OR_BOTH(bridge1, bridge2);
return AST_BRIDGE_TRANSFER_FAIL;
}
BRIDGE_LOCK_ONE_OR_BOTH(bridge1, bridge2);
if (bridge2) {
RAII_VAR(struct ast_channel *, local_chan2, NULL, ao2_cleanup);

@ -2560,7 +2560,27 @@ static void bridge_channel_event_join_leave(struct ast_bridge_channel *bridge_ch
ao2_iterator_destroy(&iter);
}
int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
void bridge_channel_internal_wait(struct bridge_channel_internal_cond *cond)
{
ast_mutex_lock(&cond->lock);
while (!cond->done) {
ast_cond_wait(&cond->cond, &cond->lock);
}
ast_mutex_unlock(&cond->lock);
}
void bridge_channel_internal_signal(struct bridge_channel_internal_cond *cond)
{
if (cond) {
ast_mutex_lock(&cond->lock);
cond->done = 1;
ast_cond_signal(&cond->cond);
ast_mutex_unlock(&cond->lock);
}
}
int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel,
struct bridge_channel_internal_cond *cond)
{
int res = 0;
struct ast_bridge_features *channel_features;
@ -2590,6 +2610,7 @@ int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
bridge_channel->bridge->uniqueid,
bridge_channel,
ast_channel_name(bridge_channel->chan));
bridge_channel_internal_signal(cond);
return -1;
}
ast_channel_internal_bridge_set(bridge_channel->chan, bridge_channel->bridge);
@ -2624,6 +2645,8 @@ int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
}
bridge_reconfigured(bridge_channel->bridge, !bridge_channel->inhibit_colp);
bridge_channel_internal_signal(cond);
if (bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT) {
/*
* Indicate a source change since this channel is entering the

Loading…
Cancel
Save