asterisk users - Aug 2017 - Bug in main/bridge.c:ast_bridge_update_talker_src_video_mode

I've had two Asterisk crashes today that seem to be caused by errors
where chan->tech_pvt is pointing to something that can't be deallocated
and I think I see a reference count bug in the above function.

It contains:

                if (data->chan_old_vsrc) {
                        ast_channel_unref(data->chan_old_vsrc);
                }

Shouldn't this also have:

			data->chan_old_vsrc = NULL;

It seems to me that if it doesn't and the next condition also isn't
true, then the next time this same code is executed, it'll decrement
the reference count of the old channel again, which is wrong since it
hasn't been decremented.

What am I missing?

On Mon, Aug 28, 2017 at 6:35 PM, Richard Kenner <kenner at gnat.com>
wrote:
> I've had two Asterisk crashes today that seem to be caused by errors
> where chan->tech_pvt is pointing to something that can't be
deallocated
> and I think I see a reference count bug in the above function.
>
> It contains:
>
>                 if (data->chan_old_vsrc) {
>                         ast_channel_unref(data->chan_old_vsrc);
>                 }
>
> Shouldn't this also have:
>
>                         data->chan_old_vsrc = NULL;
>
> It seems to me that if it doesn't and the next condition also isn't
> true, then the next time this same code is executed, it'll decrement
> the reference count of the old channel again, which is wrong since it
> hasn't been decremented.
>
Yes, doing that would be a good thing.  What you point out does leave a
dangling
channel pointer in data->chan_old_vsrc if the pointer is not set to NULL.
Please
create an issue for the dangling pointer.  The patch needs to be done for
v13+.

Richard
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.digium.com/pipermail/asterisk-users/attachments/20170828/3b2340dc/attachment.html>