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

2 files changed, 68 insertions, 8 deletions

diff --git a/main/bridge.c b/main/bridge.c
index 75cd671bf..7644884eb 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -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);
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index b7f0ba51a..3ee04db55 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -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