[Spice-devel] [PATCH 3/4] channel-display-gst: Don't unref appsink and pipeline objects

[Frediano Ziglio]

Il giorno ven 15 set 2023 alle ore 01:33 Vivek Kasireddy
<vivek.kasireddy at intel.com> ha scritto:
>
> It looks like setting the Gst pipeline state to GST_STATE_NULL
> would cause all the references on most of the objects associated
> with the pipeline to be dropped; therefore, there is no need
> to explicitly unref them while freeing the pipeline. This patch
> prevents the following errors from showing up when remote-viewer
> is closed:
>
> (remote-viewer:64344): GStreamer-CRITICAL **: 00:31:04.278:
> Trying to dispose object "appsink0", but it still has a parent
> "pipeline0".
> You need to let the parent manage the object instead of unreffing the
> object directly.
>
> [MOS]:  CRITICAL - mos_bo_unreference:166: Input null ptr
>
> [MOS]:  CRITICAL - mos_bo_unreference:166: Input null ptr
>
> Cc: Frediano Ziglio <freddy77 at gmail.com>
> Cc: Dongwon Kim <dongwon.kim at intel.com>
> Cc: Jin Chung Teng <jin.chung.teng at intel.com>
> Cc: Hazwan Arif Mazlan <hazwan.arif.mazlan at intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> ---
>  src/channel-display-gst.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 3f46a65..ad0ac99 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -353,10 +353,6 @@ static void free_pipeline(SpiceGstDecoder *decoder)
>
>      gst_element_set_state(decoder->pipeline, GST_STATE_NULL);
>      gst_object_unref(decoder->appsrc);
> -    if (decoder->appsink) {
> -        gst_object_unref(decoder->appsink);
> -    }
> -    gst_object_unref(decoder->pipeline);

That's pretty odd. It looks like we are inserting a leak. Usually if
you call gst_object_unref once more it causes an error like either a
double free in glibc or an assert error in gstreamer. I'm sure we used
this path so this should have happened.
And to be honest reading the error it seems to suggest a different
issue, like the appsink is still attached to the pipeline.
Maybe unrefing pipeline before appsink fix the issue?
I personally didn't see this error before.

>      gst_object_unref(decoder->clock);
>      decoder->pipeline = NULL;
>  }

Frediano