<p> Attention is currently required from: N A. </p>
<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/18577">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File apps/app_confbridge.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18577/comment/60747f7b_19390ca8">Patch Set #1, Line 1742:</a> <code style="font-family:monospace,monospace"> conference = ao2_find(conference_bridges, conference_name, OBJ_KEY);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">The underlying issue here is that the conference bridge has transitioned to empty and another thread has not yet called/completed conf_ended.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think instead if conference is retrieved here and the state of it is empty, then conf_ended needs to be executed within this thread to remove the conference bridge from the container, send the proper conference end event, and to stop the recording. This ensures that things make sense for the newly created conference bridge. Failure to do this could result in out of order events at the very least.</p><p style="white-space: pre-wrap; word-wrap: break-word;">The conf_ended function also needs to handle being called twice. If called twice then the second time needs to be a noop. This can be achieved by returning early if ao2_unlink fails.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18577/comment/b1065155_6ffb854e">Patch Set #1, Line 1906:</a> <code style="font-family:monospace,monospace"> /* Rare, but if threads interleave exactly right, the bridge could disappear</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think this is a complete fix. I think it just reduces the chance of this happening, it doesn't eliminate it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18577/comment/d6bc8173_c46b4866">Patch Set #1, Line 1915:</a> <code style="font-family:monospace,monospace"> conference->bridge, conference2 ? "changed" : "disappeared");</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">It's not possible for conference->bridge to be NULL here.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18577">change 18577</a>. To unsubscribe, or for help writing mail filters, 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/c/asterisk/+/18577"/><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-Change-Id: I08a440eafbf83ec4b502d1e44c3f4d44c4a522f9 </div>
<div style="display:none"> Gerrit-Change-Number: 18577 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 31 May 2022 11:14:20 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>