From 7528b86cad649567f7e4b159e593c6def5198212 Mon Sep 17 00:00:00 2001
From: Joshua Colp <jcolp@digium.com>
Date: Fri, 20 Apr 2018 23:12:53 +0000
Subject: [PATCH] stream: Make the topology a reference counted object.

The stream topology has no lock of its own resulting in
another lock protecting it in some way (for example the
channel lock). If multiple channels are being juggled at
the same time this can be problematic. This change makes
the topology a reference counted object instead which
guarantees it will remain valid even without the channel
lock being held.

Change-Id: I4f4d3dd856a033ed55fe218c3a4fab364afedb03
---
 bridges/bridge_softmix.c  | 10 +---------
 include/asterisk/stream.h |  6 +++++-
 main/stream.c             | 20 +++++++++++---------
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index 23dc548467..46b27f1042 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -2159,15 +2159,7 @@ static void softmix_bridge_stream_topology_changed(struct ast_bridge *bridge, st
 		ast_bridge_channel_lock(participant);
 		ast_channel_lock(participant->chan);
 
-		topology = ast_channel_get_stream_topology(participant->chan);
-		if (topology) {
-			/*
-			 * Sigh.  We have to clone to avoid deadlock in
-			 * map_source_to_destinations() because topology
-			 * is not an ao2 object.
-			 */
-			topology = ast_stream_topology_clone(topology);
-		}
+		topology = ao2_bump(ast_channel_get_stream_topology(participant->chan));
 		if (!topology) {
 			/* Oh, my, we are in trouble. */
 			ast_channel_unlock(participant->chan);
diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h
index 0a5550b5ee..ade740d19d 100644
--- a/include/asterisk/stream.h
+++ b/include/asterisk/stream.h
@@ -306,6 +306,8 @@ void ast_stream_set_rtp_codecs(struct ast_stream *stream, struct ast_rtp_codecs
  * \retval NULL failure
  *
  * \since 15
+ *
+ * \note This returns an ao2 refcounted object
  */
 struct ast_stream_topology *ast_stream_topology_alloc(void);
 
@@ -318,6 +320,8 @@ struct ast_stream_topology *ast_stream_topology_alloc(void);
  * \retval NULL failure
  *
  * \since 15
+ *
+ * \note This returns an ao2 refcounted object
  */
 struct ast_stream_topology *ast_stream_topology_clone(
 	const struct ast_stream_topology *topology);
@@ -337,7 +341,7 @@ int ast_stream_topology_equal(const struct ast_stream_topology *left,
 	const struct ast_stream_topology *right);
 
 /*!
- * \brief Destroy a stream topology
+ * \brief Unreference and destroy a stream topology
  *
  * \param topology The topology of streams
  *
diff --git a/main/stream.c b/main/stream.c
index dd3e765f64..47415bffff 100644
--- a/main/stream.c
+++ b/main/stream.c
@@ -344,18 +344,26 @@ void ast_stream_set_rtp_codecs(struct ast_stream *stream, struct ast_rtp_codecs
 	stream->rtp_codecs = rtp_codecs;
 }
 
+static void stream_topology_destroy(void *data)
+{
+	struct ast_stream_topology *topology = data;
+
+	AST_VECTOR_CALLBACK_VOID(&topology->streams, ast_stream_free);
+	AST_VECTOR_FREE(&topology->streams);
+}
+
 #define TOPOLOGY_INITIAL_STREAM_COUNT 2
 struct ast_stream_topology *ast_stream_topology_alloc(void)
 {
 	struct ast_stream_topology *topology;
 
-	topology = ast_calloc(1, sizeof(*topology));
+	topology = ao2_alloc_options(sizeof(*topology), stream_topology_destroy, AO2_ALLOC_OPT_LOCK_NOLOCK);
 	if (!topology) {
 		return NULL;
 	}
 
 	if (AST_VECTOR_INIT(&topology->streams, TOPOLOGY_INITIAL_STREAM_COUNT)) {
-		ast_free(topology);
+		ao2_ref(topology, -1);
 		topology = NULL;
 	}
 
@@ -440,13 +448,7 @@ int ast_stream_topology_equal(const struct ast_stream_topology *left,
 
 void ast_stream_topology_free(struct ast_stream_topology *topology)
 {
-	if (!topology) {
-		return;
-	}
-
-	AST_VECTOR_CALLBACK_VOID(&topology->streams, ast_stream_free);
-	AST_VECTOR_FREE(&topology->streams);
-	ast_free(topology);
+	ao2_cleanup(topology);
 }
 
 int ast_stream_topology_append_stream(struct ast_stream_topology *topology, struct ast_stream *stream)
-- 
GitLab