After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 768811 - decodebin3/playbin3: fix leaks
decodebin3/playbin3: fix leaks
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 1.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-07-14 14:48 UTC by Guillaume Desmottes
Modified: 2016-07-18 13:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
decodebin3: fix collection leak (901 bytes, patch)
2016-07-14 14:49 UTC, Guillaume Desmottes
committed Details | Review
parsebin: fix fallback_collection leak (1.04 KB, patch)
2016-07-14 14:49 UTC, Guillaume Desmottes
committed Details | Review
playbin3: fix stream leak (1.16 KB, patch)
2016-07-14 14:49 UTC, Guillaume Desmottes
committed Details | Review
decodebin3: fix stream leak (990 bytes, patch)
2016-07-14 14:49 UTC, Guillaume Desmottes
committed Details | Review
decobin3-parse: fix stream leaks (1.05 KB, patch)
2016-07-14 14:50 UTC, Guillaume Desmottes
committed Details | Review
parsebin: fix event leak (1.08 KB, patch)
2016-07-14 14:50 UTC, Guillaume Desmottes
committed Details | Review
decodebin3: fix event leaks (1.28 KB, patch)
2016-07-14 14:50 UTC, Guillaume Desmottes
committed Details | Review
decodebin3: fix any caps leak (1.08 KB, patch)
2016-07-14 14:50 UTC, Guillaume Desmottes
committed Details | Review
parsebin: fix caps leak (1.07 KB, patch)
2016-07-14 14:50 UTC, Guillaume Desmottes
committed Details | Review
decodebin3: fix caps leaks (4.84 KB, patch)
2016-07-14 14:50 UTC, Guillaume Desmottes
committed Details | Review
decodebin3: fix query leak (877 bytes, patch)
2016-07-18 13:18 UTC, Guillaume Desmottes
none Details | Review
decodebin3: actually check result of accept caps query (1.09 KB, patch)
2016-07-18 13:46 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2016-07-14 14:48:56 UTC
I tested playbin3 with the leaks tracer and found a bunch of leaks.
Comment 1 Guillaume Desmottes 2016-07-14 14:49:33 UTC
Created attachment 331503 [details] [review]
decodebin3: fix collection leak

The collection owned by GstDecodebin3 has to be unreffed when disposing.
Comment 2 Guillaume Desmottes 2016-07-14 14:49:40 UTC
Created attachment 331504 [details] [review]
parsebin: fix fallback_collection leak

gst_event_new_stream_collection() doesn't consume the collection passed
to it so no need to give it an extra ref.
Comment 3 Guillaume Desmottes 2016-07-14 14:49:49 UTC
Created attachment 331505 [details] [review]
playbin3: fix stream leak

The stream returned by gst_message_streams_selected_get_stream() is
reffed.
Comment 4 Guillaume Desmottes 2016-07-14 14:49:58 UTC
Created attachment 331506 [details] [review]
decodebin3: fix stream leak

MultiQueueSlot owns a ref on the active stream so it should release it
when being freed.
Comment 5 Guillaume Desmottes 2016-07-14 14:50:04 UTC
Created attachment 331507 [details] [review]
decobin3-parse: fix stream leaks

DecodebinInputStream owns ref on the active and pending stream so they
should be dropped when being freed.
Comment 6 Guillaume Desmottes 2016-07-14 14:50:10 UTC
Created attachment 331508 [details] [review]
parsebin: fix event leak

Returning GST_PAD_PROBE_HANDLED means we are taking care of unreffing
the probe info.
Comment 7 Guillaume Desmottes 2016-07-14 14:50:16 UTC
Created attachment 331509 [details] [review]
decodebin3: fix event leaks

Returning GST_PAD_PROBE_HANDLED means we are taking care of unreffing
the probe info.
Comment 8 Guillaume Desmottes 2016-07-14 14:50:21 UTC
Created attachment 331510 [details] [review]
decodebin3: fix any caps leak

The caps passed to gst_query_set_caps_result() is not transfered.
Comment 9 Guillaume Desmottes 2016-07-14 14:50:28 UTC
Created attachment 331511 [details] [review]
parsebin: fix caps leak

The caps in gst_parse_pad_stream_start_event() was either acquired using
gst_pad_get_current_caps() (which return a new ref) or explicitly
reffed.
Comment 10 Guillaume Desmottes 2016-07-14 14:50:34 UTC
Created attachment 331512 [details] [review]
decodebin3: fix caps leaks

gst_stream_get_caps() returns a reffed caps.
Comment 11 Tim-Philipp Müller 2016-07-18 13:09:33 UTC
Nice, thanks!

I've taken the liberty to squash'em a bit, hope you don't mind :)

commit 9834782afa1e5eada76cda985f301562fae1d470
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Thu Jul 14 10:33:38 2016 +0200

    playbin3: fix stream leak
    
    The stream returned by gst_message_streams_selected_get_stream() is
    reffed.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=768811

commit 83f30627cd9460157935e7e9603c60a15555967e
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Wed Jul 13 16:16:21 2016 +0200

    decodebin3: fix collection leak
    
    The collection owned by GstDecodebin3 has to be unreffed when disposing.
    
    gst_event_new_stream_collection() doesn't consume the collection passed
    to it so no need to give it an extra ref.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=768811

commit c1db195ba5b24d58158025473a632e3fe979c0cb
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Thu Jul 14 10:34:30 2016 +0200

    decodebin3: fix stream leaks
    
    MultiQueueSlot owns a ref on the active stream so it should release it
    when being freed.
    
    DecodebinInputStream owns ref on the active and pending stream so they
    should be dropped when being freed.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=768811

commit e2263673d1a4178a1ec6e5805f75f2f8fe96fa9a
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Thu Jul 14 14:24:23 2016 +0200

    decodebin3: fix event leaks
    
    Returning GST_PAD_PROBE_HANDLED means we are taking care of unreffing
Comment 12 Tim-Philipp Müller 2016-07-18 13:11:14 UTC
Comment on attachment 331510 [details] [review]
decodebin3: fix any caps leak

Solved that a bit differently (using GST_CAPS_ANY instead).
Comment 13 Guillaume Desmottes 2016-07-18 13:18:07 UTC
Created attachment 331707 [details] [review]
decodebin3: fix query leak

gst_pad_query() doesn't consume the query.
Comment 14 Guillaume Desmottes 2016-07-18 13:18:31 UTC
Sorry, I forgot to attach one patch.
Comment 15 Guillaume Desmottes 2016-07-18 13:46:59 UTC
Created attachment 331711 [details] [review]
decodebin3: actually check result of accept caps query

We were just checking if the query was handled, not its result.

Also fix a leak as gst_pad_query() was not consuming the query.
Comment 16 Tim-Philipp Müller 2016-07-18 13:57:09 UTC
Comment on attachment 331711 [details] [review]
decodebin3: actually check result of accept caps query

Thanks for updating the patch!

commit 6c58f5ee2f97baed5ca36b3504dc4e8026915f76
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Wed Jul 13 14:17:25 2016 +0200

    decodebin3: actually check result of accept caps query
    
    We were just checking if the query was handled, not its result.
    
    Also fix a leak as gst_pad_query() was not consuming the query.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=768811