> On Aug. 21, 2014, 1:30 p.m., rmudgett wrote: > > /branches/12/res/res_stasis_playback.c, lines 464-468 > > <https://reviewboard.asterisk.org/r/3908/diff/3/?file=66672#file66672line464> > > > > To be safe, swap the ao2_ref and the send_comand statements. If the > > send_command fails to send, playback is unreffed thus this function may no > > longer have a valid ref to playback. > > > > The playback RAII_VAR here looks to be another silly useage. > > > > The game played with the playback ref here is rather fragile. A > > playback ref should be passed to the play_uri command and cleaned up by the > > remove_from_playback destructor as well as unlinking playback from the > > playbacks container. What happens if playback fails to get linked into the > > playbacks container a few lines earlier?
I'd like you to take another peek at this function in particular to make sure I didn't screw anything up when I post the next diff. > On Aug. 21, 2014, 1:30 p.m., rmudgett wrote: > > /branches/12/res/stasis/control.c, lines 937-938 > > <https://reviewboard.asterisk.org/r/3908/diff/3/?file=66676#file66676line937> > > > > The line wasn't long enough to require a wrap. I'll go ahead and change this, but sometimes I like to wrap to group arguments logically rather than for line length. In this case, the data and the data destructor argument. It doesn't really seem as useful since the data destructor is NULL. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3908/#review13141 ----------------------------------------------------------- On Aug. 21, 2014, 11:30 a.m., Jonathan Rose wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3908/ > ----------------------------------------------------------- > > (Updated Aug. 21, 2014, 11:30 a.m.) > > > Review request for Asterisk Developers, kmoore, Matt Jordan, and rmudgett. > > > Bugs: ASTERISK-24147 > https://issues.asterisk.org/jira/browse/ASTERISK-24147 > > > Repository: Asterisk > > > Description > ------- > > Here's a basic rundown of what was happening: > > 1. Stasis application with a channel in it, channel is in a bridge. Playback > actions for the channel are queued, the stasis control is running them. > > 2. The channel hangs up. Because the stasis control detects a hangup and the > depart is expected to be handled when the channel leaves the bridge, the > stasis application execution function exits taking the control along with it. > When the PBX thread exits, the channel is considered fully hung up and leaves > the bridging core. At this point we are running after bridge callbacks with, > one of which holds a pointer to a dead control object that we are trying to > do all queue actions to along with other things. This causes the crash. > > In order to fix this, I made sure that if the control execution loop is > exited without pulling the channel out of the bridge that the channel depart > would occur manually and the control would be marked so that the after bridge > cb wouldn't attempt to exit the bridge in this case. This wasn't actually > causing problems, but Richard and I both have concerns about an after bridge > callback attempting to depart a channel from a bridge... it seems a little > out there. > > > Diffs > ----- > > /branches/12/res/stasis/stasis_bridge.c 421424 > /branches/12/res/stasis/control.c 421424 > /branches/12/res/stasis/command.c 421424 > /branches/12/res/stasis/command.h 421424 > /branches/12/res/res_stasis_recording.c 421424 > /branches/12/res/res_stasis_playback.c 421424 > /branches/12/res/res_stasis_answer.c 421424 > /branches/12/res/res_stasis.c 421424 > /branches/12/include/asterisk/stasis_app_impl.h 421424 > > Diff: https://reviewboard.asterisk.org/r/3908/diff/ > > > Testing > ------- > > Made sure the crash that was happening no longer occurs > > Tested long queues of playback to check what would happen with the additional > playbacks. Each subsequent playback from the one currently running fails (in > an expected manner) and reports its failure over stasis. Nothing too odd > there. > > > Thanks, > > Jonathan Rose > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
