<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6275">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">channel: Fix topology API locking.<br><br>* ast_channel_request_stream_topology_change() must not be called with any<br>channel locks held.<br><br>* ast_channel_stream_topology_changed() must be called with only the<br>passed channel lock held.<br><br>ASTERISK-27212<br><br>Change-Id: I843de7956d9f1cc7cc02025aea3463d8fe19c691<br>---<br>M include/asterisk/channel.h<br>M main/channel.c<br>M tests/test_stream.c<br>3 files changed, 14 insertions(+), 3 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h<br>index f0fe5b2..3dfbe61 100644<br>--- a/include/asterisk/channel.h<br>+++ b/include/asterisk/channel.h<br>@@ -4934,7 +4934,7 @@<br> * \param topology The new stream topology<br> * \param change_source The source that initiated the change<br> *<br>- * \pre chan is locked<br>+ * \note Absolutely _NO_ channel locks should be held before calling this function.<br> *<br> * \retval 0 request has been accepted to be attempted<br> * \retval -1 request could not be attempted<br>@@ -4956,7 +4956,7 @@<br> * \param chan The channel to provide notice to<br> * \param topology The new stream topology<br> *<br>- * \pre chan is locked<br>+ * \pre chan is locked Absolutely _NO_ other channels can be locked.<br> *<br> * \retval 0 success<br> * \retval -1 failure<br>diff --git a/main/channel.c b/main/channel.c<br>index 632d472..74de9ca 100644<br>--- a/main/channel.c<br>+++ b/main/channel.c<br>@@ -10843,22 +10843,29 @@<br> int ast_channel_request_stream_topology_change(struct ast_channel *chan,<br> struct ast_stream_topology *topology, void *change_source)<br> {<br>+ int res;<br>+<br> ast_assert(chan != NULL);<br> ast_assert(topology != NULL);<br> <br>+ ast_channel_lock(chan);<br> if (!ast_channel_is_multistream(chan) || !ast_channel_tech(chan)->indicate) {<br>+ ast_channel_unlock(chan);<br> return -1;<br> }<br> <br> if (ast_stream_topology_equal(ast_channel_get_stream_topology(chan), topology)) {<br> ast_debug(3, "Topology of %s already matches what is requested so ignoring topology change request\n",<br> ast_channel_name(chan));<br>+ ast_channel_unlock(chan);<br> return 0;<br> }<br> <br> ast_channel_internal_set_stream_topology_change_source(chan, change_source);<br> <br>- return ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, topology, sizeof(topology));<br>+ res = ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, topology, sizeof(topology));<br>+ ast_channel_unlock(chan);<br>+ return res;<br> }<br> <br> int ast_channel_stream_topology_changed(struct ast_channel *chan, struct ast_stream_topology *topology)<br>diff --git a/tests/test_stream.c b/tests/test_stream.c<br>index 28983e0..8c88704 100644<br>--- a/tests/test_stream.c<br>+++ b/tests/test_stream.c<br>@@ -1768,7 +1768,9 @@<br> ast_test_validate_cleanup(test, change_res == -1, res, done);<br> ast_test_validate_cleanup(test, !pvt->indicated_change_request, res, done);<br> <br>+ ast_channel_lock(mock_channel);<br> change_res = ast_channel_stream_topology_changed(mock_channel, topology);<br>+ ast_channel_unlock(mock_channel);<br> <br> ast_test_validate_cleanup(test, change_res == -1, res, done);<br> ast_test_validate_cleanup(test, !pvt->indicated_changed, res, done);<br>@@ -1876,7 +1878,9 @@<br> ast_test_validate_cleanup(test, !change_res, res, done);<br> ast_test_validate_cleanup(test, pvt->indicated_change_request, res, done);<br> <br>+ ast_channel_lock(mock_channel);<br> change_res = ast_channel_stream_topology_changed(mock_channel, topology);<br>+ ast_channel_unlock(mock_channel);<br> <br> ast_test_validate_cleanup(test, !change_res, res, done);<br> ast_test_validate_cleanup(test, pvt->indicated_changed, res, done);<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6275">change 6275</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6275"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I843de7956d9f1cc7cc02025aea3463d8fe19c691 </div>
<div style="display:none"> Gerrit-Change-Number: 6275 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>