[Asterisk-code-review] stream: Make the topology a reference counted object. (asterisk[master])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/8927 )

Change subject: stream: Make the topology a reference counted object.
......................................................................

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
---
M bridges/bridge_softmix.c
M include/asterisk/stream.h
M main/stream.c
3 files changed, 17 insertions(+), 19 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Matthew Fredrickson: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index 23dc548..46b27f1 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -2159,15 +2159,7 @@
 		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 0a5550b..ade740d 100644
--- a/include/asterisk/stream.h
+++ b/include/asterisk/stream.h
@@ -306,6 +306,8 @@
  * \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 @@
  * \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 @@
 	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 dd3e765..47415bf 100644
--- a/main/stream.c
+++ b/main/stream.c
@@ -344,18 +344,26 @@
 	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 @@
 
 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)

-- 
To view, visit https://gerrit.asterisk.org/8927
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4f4d3dd856a033ed55fe218c3a4fab364afedb03
Gerrit-Change-Number: 8927
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180508/47e1c2ce/attachment.html>