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

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index f0fe5b212fd5d9cc7f5f57af893e99ffcb743e35..3dfbe61d9061f29f56f539da832153f5ac0506ba 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4934,7 +4934,7 @@ int ast_channel_is_multistream(struct ast_channel *chan);
  * \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 @@ int ast_channel_request_stream_topology_change(struct ast_channel *chan,
  * \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 632d4724783ed149405fa3381d23161a0c0be27d..74de9caac8f66be7e2012f4a8ad4decb14c219d4 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -10843,22 +10843,29 @@ enum ast_channel_error ast_channel_errno(void)
 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 28983e05cce39b1ccc1bfc5cf57bfd96bde341d2..8c887048cfbb4360326fc9d7ad9fd38bea31bf79 100644 (file)
--- a/tests/test_stream.c
+++ b/tests/test_stream.c
@@ -1768,7 +1768,9 @@ AST_TEST_DEFINE(stream_topology_change_request_from_application_non_multistream)
        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_DEFINE(stream_topology_change_request_from_application)
        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);