Bug 214345 – [MSE][GStreamer] Break circular reference between SourceBufferPrivateGStreamer and AppendPipeline

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alicia Boya García]

SourceBufferPrivate is ref counted.

AppendPipeline is owned exclusively by SourceBufferPrivateGStreamer:
it's born and destroyed with it, managed by a never-moved unique_ptr.

AppendPipeline needs a reference to SourceBufferPrivateGStreamer to
notify it of essential events like samples having been parsed. This
used to be a Ref<>, thus creating a circular reference leak:

AppendPipeline is only destroyed in SourceBufferPrivateGStreamer
destructor. AppendPipeline holds ref counted reference to
SourceBufferPrivateGStreamer, therefore neither are destroyed.

This patch breaks the cycle by replacing the Ref<> in AppendPipeline
with a plain old reference. This is safe because
SourceBufferPrivateGStreamer owns, and therefore is alive at least
just as long as AppendPipeline.

As a consequence of not using Ref<>, the SourceBufferPrivateGStreamer
constructor does no longer need to relax the adoption requirements and
unique_ptr<AppendPipeline> can be replaced by a UniqueRef<>.

[Alicia Boya García]

Created attachment 404329 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 404329 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404329&action=review

> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h:57
> -    Ref<SourceBufferPrivateGStreamer> sourceBufferPrivate() { return m_sourceBufferPrivate.get(); }
> +    SourceBufferPrivateGStreamer& sourceBufferPrivate() { return m_sourceBufferPrivate; }
>      GstCaps* appsinkCaps() { return m_appsinkCaps.get(); }
>      RefPtr<WebCore::TrackPrivateBase> track() { return m_track; }
>      MediaPlayerPrivateGStreamerMSE* playerPrivate() { return m_playerPrivate; }

At some point I think we should flag all these that just return object state as const. I think we would need a separate bug but if you feel like doing it, it would be great.

Besides, the original code should have returned a const Ref& as you won't touch the pointer but the object in it.

[EWS]

Committed r264392: <https://trac.webkit.org/changeset/264392>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404329 [details].