[Asterisk-code-review] channel: Fix topology API locking. (asterisk[15])

[Jenkins2]

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

Change subject: channel: Fix topology API locking.
......................................................................

channel: Fix topology API locking.

* ast_channel_request_stream_topology_change() must not be called with any
channel locks held.

* ast_channel_stream_topology_changed() must be called with only the
passed channel lock held.

ASTERISK-27212

Change-Id: I843de7956d9f1cc7cc02025aea3463d8fe19c691
---
M include/asterisk/channel.h
M main/channel.c
M tests/test_stream.c
3 files changed, 14 insertions(+), 3 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index f0fe5b2..3dfbe61 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4934,7 +4934,7 @@
  * \param topology The new stream topology
  * \param change_source The source that initiated the change
  *
- * \pre chan is locked
+ * \note Absolutely _NO_ channel locks should be held before calling this function.
  *
  * \retval 0 request has been accepted to be attempted
  * \retval -1 request could not be attempted
@@ -4956,7 +4956,7 @@
  * \param chan The channel to provide notice to
  * \param topology The new stream topology
  *
- * \pre chan is locked
+ * \pre chan is locked  Absolutely _NO_ other channels can be locked.
  *
  * \retval 0 success
  * \retval -1 failure
diff --git a/main/channel.c b/main/channel.c
index 632d472..74de9ca 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -10843,22 +10843,29 @@
 int ast_channel_request_stream_topology_change(struct ast_channel *chan,
 		struct ast_stream_topology *topology, void *change_source)
 {
+	int res;
+
 	ast_assert(chan != NULL);
 	ast_assert(topology != NULL);
 
+	ast_channel_lock(chan);
 	if (!ast_channel_is_multistream(chan) || !ast_channel_tech(chan)->indicate) {
+		ast_channel_unlock(chan);
 		return -1;
 	}
 
 	if (ast_stream_topology_equal(ast_channel_get_stream_topology(chan), topology)) {
 		ast_debug(3, "Topology of %s already matches what is requested so ignoring topology change request\n",
 				ast_channel_name(chan));
+		ast_channel_unlock(chan);
 		return 0;
 	}
 
 	ast_channel_internal_set_stream_topology_change_source(chan, change_source);
 
-	return ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, topology, sizeof(topology));
+	res = ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, topology, sizeof(topology));
+	ast_channel_unlock(chan);
+	return res;
 }
 
 int ast_channel_stream_topology_changed(struct ast_channel *chan, struct ast_stream_topology *topology)
diff --git a/tests/test_stream.c b/tests/test_stream.c
index 28983e0..8c88704 100644
--- a/tests/test_stream.c
+++ b/tests/test_stream.c
@@ -1768,7 +1768,9 @@
 	ast_test_validate_cleanup(test, change_res == -1, res, done);
 	ast_test_validate_cleanup(test, !pvt->indicated_change_request, res, done);
 
+	ast_channel_lock(mock_channel);
 	change_res = ast_channel_stream_topology_changed(mock_channel, topology);
+	ast_channel_unlock(mock_channel);
 
 	ast_test_validate_cleanup(test, change_res == -1, res, done);
 	ast_test_validate_cleanup(test, !pvt->indicated_changed, res, done);
@@ -1876,7 +1878,9 @@
 	ast_test_validate_cleanup(test, !change_res, res, done);
 	ast_test_validate_cleanup(test, pvt->indicated_change_request, res, done);
 
+	ast_channel_lock(mock_channel);
 	change_res = ast_channel_stream_topology_changed(mock_channel, topology);
+	ast_channel_unlock(mock_channel);
 
 	ast_test_validate_cleanup(test, !change_res, res, done);
 	ast_test_validate_cleanup(test, pvt->indicated_changed, res, done);

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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: I843de7956d9f1cc7cc02025aea3463d8fe19c691
Gerrit-Change-Number: 6271
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170823/c12875e4/attachment.html>