Bug 218100 - Rapidly resizing a WebGL-rendered canvas leaks memory
Summary: Rapidly resizing a WebGL-rendered canvas leaks memory
Status: RESOLVED DUPLICATE of bug 217212
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Major
Assignee: Kenneth Russell
URL:
Keywords:
Depends on: 217212
Blocks:
  Show dependency tree / graph
 
Reported: 2020-10-22 14:47 PDT by Kenneth Russell
Modified: 2021-08-30 06:32 PDT (History)
3 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2020-10-22 14:47:34 PDT 
Sebastien Vandenberghe from Microsoft's Babylon.js team reports that rapidly resizing a WebGL-rendered canvas leaks significant amounts of memory:
https://jsfiddle.net/kr0atxuf/2/

Safari reloads the tab after about 30 seconds because it's consuming too much memory. This happens repeatedly until the browser says it failed too many times.

This is reproducible in Safari Technology Preview 114 on macOS 10.15.7, so isn't fixed by the ANGLE backend.

Looking into Source/WebCore/platform/graphics/cocoa/WebGLLayer.mm, while the old IOSurfaces should be being cleaned up because they're held onto via std::unique_ptr, it looks like perhaps other associated graphics resources aren't being cleaned up properly.

Filing as Major severity because this will significantly impact WebGL applications' reliability.
Comment 1 Kenneth Russell 2020-10-22 16:28:35 PDT 
After preparing a patch fixing this against somewhat old WebKit sources and rebasing to top-of-tree, I found Kimmo's fix for Bug 209139 might have already fixed this. Rebuilding to see.
Comment 2 Kenneth Russell 2020-10-22 16:29:02 PDT 
Sorry, that should have been Bug 217212.
Comment 3 Kenneth Russell 2020-10-22 17:15:16 PDT 
Yes, Kimmo's fix for Bug 217212 fixed this memory leak.

I'll ask about getting that merged into the Safari 14 release branch.

*** This bug has been marked as a duplicate of bug 217212 ***
Comment 4 Kimmo Kinnunen 2020-10-22 23:13:01 PDT 
I'll dupe it to bug 217581 which is open.

Although I tried to be a bit more meticulous with the underlying gpu object references when refactoring, I don't think the original code had that significant leaks (that the new code does not, at least).

*** This bug has been marked as a duplicate of bug 217581 ***
Comment 5 Kenneth Russell 2020-10-23 18:30:02 PDT 
The original code leaked EGL pbuffers that were associated with the IOSurfaces, and it looked to me like those were somehow retaining the IOSurfaces. Fixing the leak in the old [WebGLLayer allocateIOSurfaceBackingStoreWithSize:usingAlpha:] method definitely addressed the previous leak, which also doesn't exist after the fix for Bug 217212.
Comment 6 Kimmo Kinnunen 2020-10-23 21:04:05 PDT 
> The original code leaked EGL pbuffers that were associated with the IOSurfaces, and it looked to me like those were somehow retaining the IOSurfaces

EGL pbuffers do retain the iosurfaces and unretain only in destructors. 
I'll take your word on the original code leaking pbuffers and dupe this back.

*** This bug has been marked as a duplicate of bug 217212 ***