Bug 34582

Created attachment 43680 [details]
Trivial demonstration code

Arg, this is annoying..

Re-ordering doesn't work, as the nicesrc could be restarted by the bin if someone does a state change on the pipeline (or if one adds a sink which will cause a lsot state and playing->paused->playing transitions to happen).

Also, the fact that it waits for all threads to go away is nasty..

I think the right solution is to add an explicit _stop() function that must be manually called by the user. Just like we have states in GStreamer.

That _stop() function would be exactly the real_dispose() method that we already have.

You can simulate that by running g_object_dispose(fssession); directly in your code. If that fixes the problem, I think that's what I'll do, and if you dispose of the session without calling that method first, it would do a g_critical() (just like gst elements that haven't gone to playing).

Other ideas welcome.

(In reply to comment #2)
Hi Olivier,

> Re-ordering doesn't work, as the nicesrc could be restarted by the bin if
> someone does a state change on the pipeline (or if one adds a sink which will
> cause a lsot state and playing->paused->playing transitions to happen).

Please can you describe more closely why the patch will not work? I call gst_element_set_locked_state() on the nicesrc prior setting its state to GST_STATE_NULL. According to doc this "Locks the state of an element, so state changes of the parent don't affect this element anymore." so I do not completely understand how nicesrc can be restarted after this point in code. If I understand it correctly, locking the state should isolate nicesrc from the state changes on the pipeline, so it can be without fear switched to GST_STATE_NULL state and removed.

If you have a look, the same lock state - set to NULL - remove operation is used when removing recvonly_filters later in the very same fs_nice_transmitter_free_gst_stream() function. Why it will work in this case, but not with nicesrc?

BR

Jakub

Actually, the session's internal dispose is not delayed until every thread from the conference has ended. It just checks if fs_rtp_session_dispose() is called from a GstTask-initiated thread from inside the conference. If it is a case, it creates another thread to stop the session from.

(In reply to comment #4)
> It just checks if fs_rtp_session_dispose() is called from a GstTask-initiated
> thread from inside the conference. If it is a case, it creates another thread
> to stop the session from.

Thanks for this explanation! Then, I think, the bug is in fs_rtp_conference_is_internal_thread():

gboolean
fs_rtp_conference_is_internal_thread (FsRtpConference *self)
{
  guint i;
  gboolean ret = FALSE;

  GST_OBJECT_LOCK (self);
  for (i = 0; i < self->priv->threads->len; i++)
  {
    if (g_ptr_array_index (self->priv->threads, i))
    {
      ret = TRUE;
      break;
    }
  }
  GST_OBJECT_UNLOCK (self);

  return ret;
}

I think there should be

if (g_ptr_array_index (self->priv->threads, i) == g_thread_self ())

The current function only returns FALSE when the array is full of NULLs

After I reverted my previous changes and applied this one, everything started to work correctly, no more leftover elements in GStreamer pipeline.

Yep, I came to the same conclusion and its fixed now.