[gst-devel] Re: [gst-cvs] soto gstreamer: gstreamer/ gstreamer/gst/

[Martin Soto]

Hello Ronald,

On Wed, 2004-11-24 at 13:48, Ronald S. Bultje wrote: 
> OK, so I'm not the most hardcore guy, but I looked at it anyway, and
> there is some issues, both theoretical and practical. Let me start with
> theory.

I'm glad you did look.

> You lock the mutex in the state change function and then call the
> parent_class->change_state function inside this critical code part. The
> parent might decide to do negotiation in here. You lock the mutex in
> sink_link as well. That's a potential deadlock. I know you have to lock
> because you're inside a gcond here, but this seems bad. I think we
> should lock in sink_link as well and not lock in state_change, or at
> least not when calling the parent change_state function.

OK, looking at the code, I see I can grab the lock in a more granular
way in gst_queue_link_sink, namely, only when the queue has material in
it. This should at least reduce the risk of deadlock. I'll commit a fix
in a few minutes.

The more general reentrancy issue however is sort of complex. To manage
it properly, we would need something like a GStaticRecMutex, but you
cannot use condition variables with one of those. For the moment, I hope
GstElement doesn't start doing funny stuff.

> Now practical: chained ogg in totem no longer works. :). Wim believes
> it's related to you waiting for the queue to drain while it's not
> running. That's another deadlock.

Is Wim's belief somehow confirmed? (as in, remove the patch, test
again?) The new code will wait only if:

  1. The queue isn't empty.
  2. The upstream end tries to renegotiate the pad.
  3. The new caps are actually different from the old ones.

Even if all of these happen, the program is not guaranteed to block. It
will block if

  4. The queue is not in running state.
  5. It won't ever go to running state (maybe because someone is waiting
for the negotiation to happen in order to start the queue, ughh).

I don't know the internals of Totem. Is it possible for all of these
condition to meet? If that was possible before, the negotiation would
have failed, putting the pipeline in error state. Does Totem actually
recover from that?

Anyway, if you can tell me how to reproduce the problem, I'll try to
look into it.

Cheers,

M. S.