[Bug 797251] webrtcbin: might leak resources when changing to NULL

[GStreamer (GNOME Bugzilla)]

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

--- Comment #4 from Aleix Conchillo Flaqué <aconchillo at gmail.com> ---
(In reply to Matthew Waters (ystreet00) from comment #3)
> Review of attachment 373849 [details] [review]:
> 
> I'm not sure this is needed or what it changes for you?
> 

So far, the patch fixes the leaks.

> Execution of tasks are already blocked on webtrtc->priv->is_closed which is
> set to TRUE when the peerconnection thread is shutdown on _dispose().  Can
> you check that gst_webrtc_bin_dispose() is being called for you?  If not,
> that indicates a leak of the element itself.
> 

The problem is that the idle sources (whatever is executed in their callbacks)
still keep references to webrtcbin when the pipeline is being set to NULL and
unrefed. So, the element is never disposed therefore the thread doesn't
terminate and is_closed keeps being TRUE.

That is, nothing prevents the idle sources to keep executing when the element
changes state to NULL. In my app I have a ref to the pipeline and a ref to the
webrtcbin element. I set the pipeline to NULL and I unref it next. At this
point I would expect GST_OBJECT_REFCOUNT_VALUE for the webrtcbin element to
always be 1 (my copy should be the last one), but it's not.

> ::: ext/webrtc/gstwebrtcbin.c
> @@ +627,3 @@
>    PC_LOCK (op->webrtc);
> +  if (!op->webrtc->priv->running) {
> +    GST_DEBUG_OBJECT (op->webrtc, "Element is not running, aborting
> execution");
> 
> Do the unit tests pass with this addition?
> 

I'm trying now and just ran into a weird problem...

fatal: making test-suite.log: failed to create webrtcbin.trs
fatal: making test-suite.log: failed to create webrtcbin.log

First time I see this. Checking...

> A lot of the media tests perform the entire test in the NULL state which
> seems like this would prevent.
> 
> @@ +3784,3 @@
>      case GST_STATE_CHANGE_PAUSED_TO_READY:
>        webrtc->priv->running = FALSE;
> +      PC_LOCK (webrtc);
> 
> This lock should probably one line up protecting the running = FALSE.

Ooops. Fixed.

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