Bug 109350 - Resource leak related to gstreamer and videos
: Resource leak related to gstreamer and videos
Status: RESOLVED FIXED
: WebKit
Media Elements
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
: HTML5
:
: 117354
  Show dependency treegraph
 
Reported: 2013-02-09 01:48 PST by
Modified: 2014-01-17 03:08 PST (History)


Attachments
liferea thread debug info (19.05 KB, text/plain)
2013-02-09 01:48 PST, Christian Stadelmann
no flags Details
Patch (2.15 KB, patch)
2013-05-21 09:28 PST, Allan Sandfeld Jensen
no flags Review Patch | Details | Formatted Diff | Diff
Patch (1.85 KB, patch)
2013-08-05 05:22 PST, Allan Sandfeld Jensen
no flags Review Patch | Details | Formatted Diff | Diff
Patch (1.63 KB, patch)
2013-08-07 05:04 PST, Allan Sandfeld Jensen
eric.carlson: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2013-02-09 01:48:25 PST
Created an attachment (id=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/
------- Comment #1 From 2013-02-09 01:58:48 PST -------
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.
------- Comment #2 From 2013-02-14 06:43:58 PST -------
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.
------- Comment #3 From 2013-02-14 06:46:56 PST -------
http://trac.webkit.org/changeset/128298
------- Comment #4 From 2013-03-01 11:30:19 PST -------
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.
------- Comment #5 From 2013-03-02 03:37:48 PST -------
(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.
------- Comment #6 From 2013-03-02 03:52:07 PST -------
(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.
------- Comment #7 From 2013-03-02 04:03:35 PST -------
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…
------- Comment #8 From 2013-03-02 04:22:50 PST -------
(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 :)
------- Comment #9 From 2013-04-30 09:42:48 PST -------
Webkit 2.0.1, webkit2, midori, too. That thing hasn't been fixed for a single day since HTML5 support was first introduced.
------- Comment #10 From 2013-04-30 09:43:31 PST -------
*** Bug 115411 has been marked as a duplicate of this bug. ***
------- Comment #11 From 2013-05-13 03:11:01 PST -------
*** Bug 114165 has been marked as a duplicate of this bug. ***
------- Comment #12 From 2013-05-15 13:03:26 PST -------
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.
------- Comment #13 From 2013-05-17 08:04:56 PST -------
Since this was merged with similar Qt bugs, this is a general GStreamer problem.
------- Comment #14 From 2013-05-17 08:10:34 PST -------
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.
------- Comment #15 From 2013-05-17 09:43:30 PST -------
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.
------- Comment #16 From 2013-05-17 09:57:40 PST -------
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.
------- Comment #17 From 2013-05-17 10:55:53 PST -------
(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.
------- Comment #18 From 2013-05-21 09:28:51 PST -------
Created an attachment (id=202444) [details]
Patch
------- Comment #19 From 2013-05-21 11:37:45 PST -------
(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).

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.
------- Comment #20 From 2013-05-21 11:39:32 PST -------
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.
------- Comment #21 From 2013-05-21 12:33:14 PST -------
(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.

> 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.
------- Comment #22 From 2013-05-21 12:47:59 PST -------
(In reply to comment #21)
> (In reply to comment #19)
> > (From update of attachment 202444 [details] [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.
------- Comment #23 From 2013-05-21 12:49:54 PST -------
Additionally, this would not solve the problem when web pages create and use unattached HTMLAudioElements to play audio, which is very common.
------- Comment #24 From 2013-05-21 13:46:31 PST -------
(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.
------- Comment #25 From 2013-05-22 01:24:12 PST -------
(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?
------- Comment #26 From 2013-08-05 05:22:24 PST -------
Created an attachment (id=208119) [details]
Patch

Another approach by solving the general JS leak on closing webviews
------- Comment #27 From 2013-08-05 08:15:18 PST -------
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.
------- Comment #28 From 2013-08-07 05:04:25 PST -------
Created an attachment (id=208252) [details]
Patch
------- Comment #29 From 2013-08-08 09:08:22 PST -------
(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().
------- Comment #30 From 2013-08-08 09:21:19 PST -------
(In reply to comment #29)
> (From update of attachment 208252 [details] [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.
------- Comment #31 From 2013-08-09 11:13:16 PST -------
(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
>>> +    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.
------- Comment #32 From 2013-08-12 03:21:01 PST -------
Committed r153937: <http://trac.webkit.org/changeset/153937>
------- Comment #33 From 2013-08-21 07:23:02 PST -------
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.
------- Comment #34 From 2014-01-17 02:02:55 PST -------
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
------- Comment #35 From 2014-01-17 03:08:02 PST -------
(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"