[Bug 794290] rtponviftimestamp: race condition in buffer handling leading to memory corruption

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=794290

Pavel Modilaynen <pavel.modilaynen at axis.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #369720|0                           |1
        is obsolete|                            |

--- Comment #9 from Pavel Modilaynen <pavel.modilaynen at axis.com> ---
Created attachment 369758
  --> https://bugzilla.gnome.org/attachment.cgi?id=369758&action=edit
proposed fix (updated 2)

I have updated my patch adding protection for event queue as well.

> How about adding:
> ... to both send_cached_buffers_and_events() and 
> purge_cached_buffers_and_events() and use the local variables in the rest of 
> those functions. The cached items are expected to be removed once any of 
> those functions has been called.

I am not sure what would we gain from it? Purge now is fully under lock, is it
some performance penalty that you are concerned of - then I don't think we'd
win much here - purge is called only on events.

>  event_queue = self->event_queue;
>  self->event_queue = NULL;

I think we cannot just NULL-ify, since it is used without NULL checks in other
parts of code. What I did is adding temporary queue, pushing events to that
queue and clearing event_queue so that it becomes empty. All above under lock.
I hope this is appropriate. 

> The patch should also solve:
> * purge all events in event_queue in the case pad_push fails in
> send_cached_buffers_and_events()

Do you mean when handle_and_push_buffer/handle_and_push_buffer_list fails? If
yes, I addressed this situation in this patch, good point! 

> * Perhaps move purge_cached_buffers_and_events() to be called after
> change_state has chained up to the parent (Otherwise a new buffer or list
> might be cached and never free'd).

Yes, I noticed we have huge leaks here. I will address that in my new patch for
https://bugzilla.gnome.org/show_bug.cgi?id=794353

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.