Bug 109350 – Resource leak related to gstreamer and videos

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Christian Stadelmann]

Created attachment 187422 [details]
liferea thread debug info

Running Epiphany 3.4.3 on Fedora 17 amd64 and Liferea 1.10-RC1, both using
WebKitGTK+ (Version 1.8.3-1, WebKit 535.22) as browser engine.

Problem description:
The applications do not close/release all resources needed when playing HTML5
videos (i.e. videos not using Adobe flash).

Steps to reproduce (for Epiphany and Liferea):
 1. Watch multiple youtube videos (without flash player)
 2. close all tabs (for Epiphany: except one) but keep application running
 3. Open GNOME sound menu / GNOME control center | audio: It will list many
instances of Epiphany/Liferea making audio output. htop and similar tools will
list _many_ subprocesses/threads of epiphany/liferea running. Overall memory
usage of Epiphany/Liferea is big.

Expected behavior:
WebKitGTK+ should release all resources when closing related tabs

Attachment: dgb-threads from Liferea 1.10-RC1, source:
https://sourceforge.net/p/liferea/bugs/1055/

[Christian Stadelmann]

I can not reproduce this bug with Chromium 23.0.1271.95 (WebKit 537.11). The reason may be WebKit2 holding every tab in a separate process so everything gets cleaned up when closing a tab.

[Philippe Normand]

WebKitGTK+ 1.8 is getting a bit old, any way you can try a more recent release?

FYI, GStreamer is using threads quite a lot, it's normal to see various threads running.

About the "leak" you seem to experience, we recently made the player clean its pipeline after the media has been played, I can dig up the exact commit if you want.

[Philippe Normand]

http://trac.webkit.org/changeset/128298

[Christian Stadelmann]

This may be a duplicate of #89122, but I am not sure about that.

Problem still persists in Fedora 18 with webkitgtk3 1.10.2-2 and Epiphany 3.6.1.

[Philippe Normand]

(In reply to comment #1)
> I can not reproduce this bug with Chromium 23.0.1271.95 (WebKit 537.11). The reason may be WebKit2 holding every tab in a separate process so everything gets cleaned up when closing a tab.

BTW Chromium does not use WebKit2 nor the GStreamer backend.

[Philippe Normand]

(In reply to comment #4)
> This may be a duplicate of #89122, but I am not sure about that.
> 
> Problem still persists in Fedora 18 with webkitgtk3 1.10.2-2 and Epiphany 3.6.1.

I've just checked here that after a Youtube video has been played the browser application no longer appears as a Pulseaudio client.

WebKitGTK+ 1.10.2 should work like this because the patch from bug 89122 was merged in that release.

[Christian Stadelmann]

Did you make sure it was not playing from adobe flash? Did you try some more videos? 

I don't really understand why I can reproduce this bug with 1.10.2 and you can't…

[Philippe Normand]

(In reply to comment #7)
> Did you make sure it was not playing from adobe flash? Did you try some more videos? 
> 

Of course I made sure to use HTML5 video :)

[ManDay]

Webkit 2.0.1, webkit2, midori, too. That thing hasn't been fixed for a single day since HTML5 support was first introduced.

[ManDay]

*** Bug 115411 has been marked as a duplicate of this bug. ***

[Philippe Normand]

*** Bug 114165 has been marked as a duplicate of this bug. ***

[Jonathon Jongsma (jonner)]

I can reproduce on libwebkitgtk 1.11.91 with epiphany 3.7.91 (on debian unstable).  I have only a tab with the start page open in epiphany, and I still see 6 sliders for the "Web" application in the gnome volume control window.

[Allan Sandfeld Jensen]

Since this was merged with similar Qt bugs, this is a general GStreamer problem.

[Allan Sandfeld Jensen]

I have been told by a user, that the issue can be worked around by calling setHtml() on the view before exiting. So apparently the clean-up is done correctly on loading a new page, but is missing on closing of a view.

[Allan Sandfeld Jensen]

The resources are released by ~MediaPlayer, but that is not called until HTMLMediaElement is deleted, and HTMLMediaElement is a garbage collected element, which may not be garbaged collected immediately on exit, and possibly not triggered to be garbage collected unless the timer based sweeping garbage collector is implemented on the platform.

It could possibly be solved by triggering garbage collection on closing of webviews, but I feel the mediaplayer resources should have been released earlier. Perhaps when the render element is removed, and not when the DOM element is garbage collected.

[Philippe Normand]

Seems like it might be a bug on youtube side, see bug 115693
I think that GC can be prevented in cases where the element is referenced by another one.

[Xabier Rodríguez Calvar]

(In reply to comment #16)
> Seems like it might be a bug on youtube side, see bug 115693
> I think that GC can be prevented in cases where the element is referenced by another one.

I can reproduce it with MiniBrowser with just a plain web containing a video tag.

1. Open MiniBrowser with a plain web containing a video
2. Open another web, the resource is still there. It should be gone already.
3. Open the video page again, it creates another resource so we have two now.

[Allan Sandfeld Jensen]

Created attachment 202444 [details]
Patch

[Jer Noble]

Comment on attachment 202444 [details]
Patch

I don't believe this is the correct fix.  This will cause all state for the media element to be lost whenever the element is removed from the dom (or hidden with CSS).

Instead, this should be solved by running GC more often.  I'm very skeptical that an unreferenced HTMLMediaElement will survive its associated WebView being closed.

[Jer Noble]

In a separate bug, I'm looking at encouraging GC by using the "reportExtraMemoryCost()" method;  I'll pipe this down through MediaPlayerPrivate, and GTK+ can return whatever value it believes is appropriate.  Given how much memory resources media decoding uses up, this should cause the GC to run much more frequently, and cull dead HTMLMediaElements.

[Allan Sandfeld Jensen]

(In reply to comment #19)
> (From update of attachment 202444 [details])
> I don't believe this is the correct fix.  This will cause all state for the media element to be lost whenever the element is removed from the dom (or hidden with CSS).
> 
This only happens when the DOM element is removed from its document or its document destoyed, not when hidden by CSS, not even when you browse away from the page. It takes removing the element or closing the webview to detach an element.

> Instead, this should be solved by running GC more often.  I'm very skeptical that an unreferenced HTMLMediaElement will survive its associated WebView being closed.

No, the problem is not the memory, the problem is freeing unique system resources, it needs to happen right away.


(In reply to comment #20)
> In a separate bug, I'm looking at encouraging GC by using the "reportExtraMemoryCost()" method;  I'll pipe this down through MediaPlayerPrivate, and GTK+ can return whatever value it believes is appropriate.  Given how much memory resources media decoding uses up, this should cause the GC to run much more frequently, and cull dead HTMLMediaElements.

I have been using a patch to trigger extra garbage collection on the destruction of QWebViews, while this does eventually delete the HTMLMediaElement, it may wait half a minute before doing so. This is not a problem resource-wise. The problem is the video-overlay is still showing and the input is still buffering. The MediaPlayer is only paused.

Note also this is not a GTK+ specific bug.

[Jer Noble]

(In reply to comment #21)
> (In reply to comment #19)
> > (From update of attachment 202444 [details] [details])
> > I don't believe this is the correct fix.  This will cause all state for the media element to be lost whenever the element is removed from the dom (or hidden with CSS).
> > 
> This only happens when the DOM element is removed from its document or its document destoyed, not when hidden by CSS, not even when you browse away from the page. It takes removing the element or closing the webview to detach an element.

That's not true, and moreover it's besides the point.  Doing this on detach would cause the mediaPlayer to be thrown away when moving the media element around in the DOM, it would mean an element couldn't play once removed from the DOM, and it would cause the media to get thrown away when entering full screen (and that's just off of the top of my head).  This approach is wrong.

> > Instead, this should be solved by running GC more often.  I'm very skeptical that an unreferenced HTMLMediaElement will survive its associated WebView being closed.
> 
> No, the problem is not the memory, the problem is freeing unique system resources, it needs to happen right away.

Whether or not that's true is immaterial.  Doing it in detach() is wrong.

> (In reply to comment #20)
> > In a separate bug, I'm looking at encouraging GC by using the "reportExtraMemoryCost()" method;  I'll pipe this down through MediaPlayerPrivate, and GTK+ can return whatever value it believes is appropriate.  Given how much memory resources media decoding uses up, this should cause the GC to run much more frequently, and cull dead HTMLMediaElements.
> 
> I have been using a patch to trigger extra garbage collection on the destruction of QWebViews, while this does eventually delete the HTMLMediaElement, it may wait half a minute before doing so.
> 
> This is not a problem resource-wise. The problem is the video-overlay is still showing and the input is still buffering. The MediaPlayer is only paused.
> 
> Note also this is not a GTK+ specific bug.

I do realize that, and as I've stated, I'm working on the same problem in the Mac port. But if the underlying problem is that a dead object wrapper is not being GC'd when closing its associated view, then perhaps you should be looking at the GC, and not at the HTMLMediaElement.

[Jer Noble]

Additionally, this would not solve the problem when web pages create and use unattached HTMLAudioElements to play audio, which is very common.

[Allan Sandfeld Jensen]

(In reply to comment #22)
> I do realize that, and as I've stated, I'm working on the same problem in the Mac port. But if the underlying problem is that a dead object wrapper is not being GC'd when closing its associated view, then perhaps you should be looking at the GC, and not at the HTMLMediaElement.

I did look into GC. It doesn't seem like JSC GC is designed to deal with this sort of issue. It is very reactionary, cleaning up only when it needs to, or when new allocations are being made. Since it is so passive it may end up never being called for a closed webview.

[Allan Sandfeld Jensen]

(In reply to comment #22)
>  this on detach would cause the mediaPlayer to be thrown away when moving the media element around in the DOM, it would mean an element couldn't play once removed from the DOM, and it would cause the media to get thrown away when entering full screen (and that's just off of the top of my head).  This approach is wrong.
> 
I still prefer a method to stop the media when the hosting Document/Page dies. If we can not tie it to being attached to the document, I wonder if we could extend MediaCanStartListener, to include a method to also tell when to stop?

[Allan Sandfeld Jensen]

Created attachment 208119 [details]
Patch

Another approach by solving the general JS leak on closing webviews

[Allan Sandfeld Jensen]

While testing my patch I noticed the issue seems to have already been resolved mostly in a recent patch. I think it is r152778 for bug #73743 that fixed it. The underlying problem was the RefPtr<Frame> that WebKitWebSourceGStreamer held, that created a circular reference.

Of course this means now that it can actually be garbage collected, not that it will happen immediately.

[Allan Sandfeld Jensen]

Created attachment 208252 [details]
Patch

[Eric Carlson]

Comment on attachment 208252 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:4183
> +    // Once an active DOM object has been stopped it can not be restarted, so we can deallocate the media player now.
> +    m_player.clear();

This is unnecessary. Earlier in this method we called userCancelledLoad(), which called clearMediaPlayer(), which called m_player.clear().

[Allan Sandfeld Jensen]

(In reply to comment #29)
> (From update of attachment 208252 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208252&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.cpp:4183
> > +    // Once an active DOM object has been stopped it can not be restarted, so we can deallocate the media player now.
> > +    m_player.clear();
> 
> This is unnecessary. Earlier in this method we called userCancelledLoad(), which called clearMediaPlayer(), which called m_player.clear().

Only if the media file is not fully loaded. If the file is fully loaded userCancelledLoad() does nothing.

[Eric Carlson]

Comment on attachment 208252 [details]
Patch

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

>>> Source/WebCore/html/HTMLMediaElement.cpp:4183
>>> +    m_player.clear();
>> 
>> This is unnecessary. Earlier in this method we called userCancelledLoad(), which called clearMediaPlayer(), which called m_player.clear().
> 
> Only if the media file is not fully loaded. If the file is fully loaded userCancelledLoad() does nothing.

True enough. It might be worth noting this in your comment so someone doesn't "optimize" it away at some point in the future.

[Allan Sandfeld Jensen]

Committed r153937: <http://trac.webkit.org/changeset/153937>

The issue is still present. Tested using QtTestBrowser and QupZilla.

Steps: Load a YouTube video page. Until the video is fully loaded, close the tab or type in another address and load it. Network monitor apps detect hi network traffic indicating that the video is still loading in the background.

However, if I open new window of the browser and close the one which had the video page opened, loading of the video in the background stops.

[Natalia]

The issue is still present. It is simple to reproduce: open your qtwebkit browser, load some html5 video and close the tab. If stop playing video manually, it does not fix this problem.
Used environment: Fedora 19, Qt 5.2.0 and Qt 5.1.0.
You can see threads by htop (which didn't realize):
aqueue:src
vqueue:src
queue:src
multiqueue:src
matroskademux:
appsrc:src

[Natalia]

(In reply to comment #34)
> The issue is still present. It is simple to reproduce: open your qtwebkit browser, load some html5 video and close the tab. If stop playing video manually, it does not fix this problem.
> Used environment: Fedora 19, Qt 5.2.0 and Qt 5.1.0.
> You can see threads by htop (which didn't realize):
> aqueue:src
> vqueue:src
> queue:src
> multiqueue:src
> matroskademux:
> appsrc:src

Sorry, I mean "didn't release threads"

[James Hilliard]

(In reply to Natalia from comment #34)
> The issue is still present. It is simple to reproduce: open your qtwebkit
> browser, load some html5 video and close the tab. If stop playing video
> manually, it does not fix this problem.
> Used environment: Fedora 19, Qt 5.2.0 and Qt 5.1.0.
> You can see threads by htop (which didn't realize):
> aqueue:src
> vqueue:src
> queue:src
> multiqueue:src
> matroskademux:
> appsrc:src

This looks a lot like what I'm seeing in https://bugs.webkit.org/show_bug.cgi?id=241549