[gst-devel] Multi-threaded queueing locking issues

[Matt Howell]

Erik Walthinsen wrote:

> On Tue, 20 Mar 2001, Matt Howell wrote:
>
> > i don't think the check in gstpad.c::change_state() is such a bad thing.  it's
> > called in the context of the thread (the pipeline?) which was instructed to
> > go to the pause state by the app, right?  it's in gstpad.c because it's pad-
> > centric...  maybe i don't understand your concern here.
> There is no pad_change_state()...  Only the element_change_state, which is
> called within the context of the thread, but only after the thread has
> woken up from within its iteration, which is the whole problem.

doh!  you're right.  i meant gst_queue_change_state().  (s/pad/queue/g)

aren't the various change_state() calls called in the context of the app?
(a different context than the pipeline / thread containers?)

> > it doesn't seem bad to use different ways to wake threads that are blocked
> > for different reasons.  if a thread is blocked because of g_cond_wait(), it
> > should be waked with g_cond_signal().  it it's blocked because of a system
> > call (e.g., blocking read) use the system kill().
> Only if we can unblock them all without killing the thread.  While every
> element must be written so it has all of its live state within its own
> object struct (so they can be shut down and restarted at the beginning
> with no problems), there's still the problem of things like filehandles
> which will be opened from thread context.  I just wrote a test that shows
> that at least with nothing else going on, it seems that filehandles stick
> around even when the thread that created them goes away.  I have no idea
> if the same is true for other kinds of files.

the only case in which a thread would have to be killed (pthread cancel) is if
it's blocked on a system call (e.g., non-blocking read).  in the more common
case of thread synchronization (where a thread is blocked because it called
g_cond_wait()), you do not have to kill the thread.

it seems the complication arises only in the blocking system call cases
(blocking read, blocking write).  if we wrote an i/o library for filters, we
could manage this problem.  these calls would be blocking as far as the
filter is concerned, but, internally, do the i/o in a non-blocking fashion.
for example,

// add to gst_element some persistent storage for file descriptors and associated
// flags... basically, only need to know if element opened for blocking i/o

int gst_open(gst_element *e, const char *pathname, int flags, mode_t mode)
{
  int fd = open(pathname, (flags & ~(O_NONBLOCK | O_NDELAY));
  if (flags & (O_NONBLOCK | O_NDELAY) && fd != -1)
  {
    // record blocking i/o for this fd in e's persistent data
  }
  return fd;
}

size_t gst_read(gst_element *e, fd, void *buf, size_t count)
{
  if ( GST_FD_FLAGS(e,fd) & (O_NONBLOCK | O_NDELAY) )
  {
    int actual = 0;
    while (count>0 && GST_STATE_PENDING(e) != STATE_PAUSED)
    {
      if ((actual = read(fd, buf, count)) < 0) return actual;
      if (actual)
        count -= actual; buf += actual;
      else
        // some timeout ... maybe usleep(100) ??
    }
  }
  else
    return read(fd, buf, count);
}

however, such an approach may break certain uses of a "blocking" call.
any thoughts?  problems?

matt.


> > the spin / timeout stuff could be encapsulated in an element's change_state().
> > (this requires that all elements that can block on a system call implement
> > their own change_state().)  as brent points out below, this could use
> > pthread_cancel() (assuming these elements are in a pthread) as a last resort
> > (since it is a bit destructive ;-).
> Remember though that change_state doesn't happen for an element until the
> thread has been unblocked anyway, because the change_state happens from
> thread context.  The thread has to be told to do the change_state...  And
> pthread_cancel() has the potential problem of blowing away per-thread
> state.
>
>       Erik Walthinsen <omega at temple-baptist.com> - System Administrator
>         __
>        /  \                GStreamer - The only way to stream!
>       |    | M E G A        ***** http://gstreamer.net/ *****
>       _\  /_
>
> _______________________________________________
> gstreamer-devel mailing list
> gstreamer-devel at lists.sourceforge.net
> http://lists.sourceforge.net/lists/listinfo/gstreamer-devel