Bug 218100

Summary:	Rapidly resizing a WebGL-rendered canvas leaks memory
Product:	WebKit	Reporter:	Kenneth Russell <kbr>
Component:	WebGL	Assignee:	Kenneth Russell <kbr>
Status:	RESOLVED DUPLICATE
Severity:	Major	CC:	dino, jdarpinian, kkinnunen
Priority:	P2
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified
See Also:	https://bugs.webkit.org/show_bug.cgi?id=219780 https://bugs.webkit.org/show_bug.cgi?id=229606
Bug Depends on:	217212
Bug Blocks:

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 ***