Bug 109350 - Resource leak related to gstreamer and videos
Summary: Resource leak related to gstreamer and videos
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords: HTML5
: 114165 115411 (view as bug list)
Depends on:
Blocks: 117354
  Show dependency treegraph
 
Reported: 2013-02-09 01:48 PST by Christian Stadelmann
Modified: 2022-07-17 20:19 PDT (History)
17 users (show)

See Also:


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 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (1.85 KB, patch)
2013-08-05 05:22 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (1.63 KB, patch)
2013-08-07 05:04 PDT, Allan Sandfeld Jensen
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Stadelmann 2013-02-09 01:48:25 PST
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/
Comment 1 Christian Stadelmann 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 Philippe Normand 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 Philippe Normand 2013-02-14 06:46:56 PST
http://trac.webkit.org/changeset/128298
Comment 4 Christian Stadelmann 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 Philippe Normand 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 Philippe Normand 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 Christian Stadelmann 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 Philippe Normand 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 ManDay 2013-04-30 09:42:48 PDT
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 ManDay 2013-04-30 09:43:31 PDT
*** Bug 115411 has been marked as a duplicate of this bug. ***
Comment 11 Philippe Normand 2013-05-13 03:11:01 PDT
*** Bug 114165 has been marked as a duplicate of this bug. ***
Comment 12 Jonathon Jongsma (jonner) 2013-05-15 13:03:26 PDT
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 Allan Sandfeld Jensen 2013-05-17 08:04:56 PDT
Since this was merged with similar Qt bugs, this is a general GStreamer problem.
Comment 14 Allan Sandfeld Jensen 2013-05-17 08:10:34 PDT
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 Allan Sandfeld Jensen 2013-05-17 09:43:30 PDT
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 Philippe Normand 2013-05-17 09:57:40 PDT
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 Xabier Rodríguez Calvar 2013-05-17 10:55:53 PDT
(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 Allan Sandfeld Jensen 2013-05-21 09:28:51 PDT
Created attachment 202444 [details]
Patch
Comment 19 Jer Noble 2013-05-21 11:37:45 PDT
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.
Comment 20 Jer Noble 2013-05-21 11:39:32 PDT
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 Allan Sandfeld Jensen 2013-05-21 12:33:14 PDT
(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.
Comment 22 Jer Noble 2013-05-21 12:47:59 PDT
(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.
Comment 23 Jer Noble 2013-05-21 12:49:54 PDT
Additionally, this would not solve the problem when web pages create and use unattached HTMLAudioElements to play audio, which is very common.
Comment 24 Allan Sandfeld Jensen 2013-05-21 13:46:31 PDT
(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 Allan Sandfeld Jensen 2013-05-22 01:24:12 PDT
(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 Allan Sandfeld Jensen 2013-08-05 05:22:24 PDT
Created attachment 208119 [details]
Patch

Another approach by solving the general JS leak on closing webviews
Comment 27 Allan Sandfeld Jensen 2013-08-05 08:15:18 PDT
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 Allan Sandfeld Jensen 2013-08-07 05:04:25 PDT
Created attachment 208252 [details]
Patch
Comment 29 Eric Carlson 2013-08-08 09:08:22 PDT
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().
Comment 30 Allan Sandfeld Jensen 2013-08-08 09:21:19 PDT
(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.
Comment 31 Eric Carlson 2013-08-09 11:13:16 PDT
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.
Comment 32 Allan Sandfeld Jensen 2013-08-12 03:21:01 PDT
Committed r153937: <http://trac.webkit.org/changeset/153937>
Comment 33 jingdow 2013-08-21 07:23:02 PDT
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 Natalia 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 Natalia 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"
Comment 36 James Hilliard 2022-07-17 20:19:46 PDT
(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