[Asterisk-code-review] app_confbridge: Fix bridge shutdown race condition. (asterisk[master])

[Joshua Colp]

Attention is currently required from: N A.
Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18577 )

Change subject: app_confbridge: Fix bridge shutdown race condition.
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

File apps/app_confbridge.c:

https://gerrit.asterisk.org/c/asterisk/+/18577/comment/60747f7b_19390ca8 
PS1, Line 1742: 	conference = ao2_find(conference_bridges, conference_name, OBJ_KEY);
The underlying issue here is that the conference bridge has transitioned to empty and another thread has not yet called/completed conf_ended.

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.

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.


https://gerrit.asterisk.org/c/asterisk/+/18577/comment/b1065155_6ffb854e 
PS1, Line 1906: 	/* Rare, but if threads interleave exactly right, the bridge could disappear
I don't think this is a complete fix. I think it just reduces the chance of this happening, it doesn't eliminate it.


https://gerrit.asterisk.org/c/asterisk/+/18577/comment/d6bc8173_c46b4866 
PS1, Line 1915: 			conference->bridge, conference2 ? "changed" : "disappeared");
It's not possible for conference->bridge to be NULL here.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18577
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I08a440eafbf83ec4b502d1e44c3f4d44c4a522f9
Gerrit-Change-Number: 18577
Gerrit-PatchSet: 1
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Comment-Date: Tue, 31 May 2022 11:14:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220531/6b5f3c19/attachment.html>