[Bug 765728] adaptivedemux: Segfault since "use realtime_clock for waiting for a condition" commit

[GStreamer (GNOME Bugzilla)]

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

--- Comment #27 from A Ashley <bugzilla at ashley-family.net> ---
(In reply to Tim-Philipp Müller from comment #23)
> Comment on attachment 327009 [details] [review]
> adaptivedemux: unschedule clock callback when tasks are ending
> 
> Nice catch. At first glance this whole function looks a bit fishy to me
> though.
> 
> There are three question I have:
> 
> 1) as I understand it, g_cond_wait() may wake up spuriously without anyone
> having called g_cond_signal() on it, so g_cond_wait() should always be used
> in a loop really. Here we probably want to do { g_cond_wait(); } while
> (!timer.fired) or so, no?
> 
> 2) This means we need to check another variable to see if we were cancelled
> and should break out of the loop.

The gst_adaptive_demux_wait_until() function is replacing a call to
g_cond_wait(). Therefore that while() { } logic should already exist in the
functions that call gst_adaptive_demux_wait_until().
> 
> 3) It's not clear to me if we can use a stack-allocated
> GstAdaptiveDemuxTimer here. The clock callbacks will be called from a
> different thread, right? So when we call gst_clock_id_unschedule() as in the
> patch we have no guarantee that the timer callback is not currently running
> in the other thread (waiting for us to release the lock perhaps), so we
> might return from the function and trash our stack before the callback gets
> a chance to finish if I'm not mistaken.

Yes, I think that is a potential problem. I've created a new patch that uses a
reference counted object.

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