search

WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
213 stars 136 forks source link

[WPE-2.38] Memory leak when images have loading="lazy" attribute #1194

Closed jakub-gocol-red closed 9 months ago

jakub-gocol-red commented 11 months ago

When images have loading="lazy" attribute, they leak memory. When those element are repeatedly recreated from scratch, it can lead to oom.

Reproduction steps

  1. Load bluebox_oom.zip test page
  2. Monitor memory consumption

Expected behaviour

Memory usage stays constant

Actual behavior

Memory usage constantly increases

Notes

It doesn't reproduce on wpe2.22, when images doesn't have loading="lazy" attribute, or when lazy loading feature is "disabled" (by editing code so this function always returns false).

pgorszkowski-igalia commented 11 months ago

@jakub-gocol-red can you check does fix from https://github.com/WebKit/WebKit/pull/18271 solve this problem?

jakub-gocol-red commented 11 months ago

yes, it solves this issue

modeveci commented 11 months ago

@pgorszkowski-igalia thank you for the patch! Can you close the ticket once it is integrated on downstream wpe-2.38? Thank you very much! Ozgur

jakub-gocol-red commented 11 months ago

After more testing it turns out there's still issue with memory leak. It doesn't reproduce on simple testcase I provided in this issue, but still reproduces on BlueTV application. But it's much lower. Without provided patch I'm getting OOM after 2 minutes of constant navigation on the application, but with the patch it takes 10 minutes to crash the browser. When I patch the browser to ignore lazy loading attribute, there is no leak.

I'll provide another simple testcase or find our if @pgorszkowski-igalia can get credentials to this application.

pgorszkowski-igalia commented 11 months ago

I am able to reproduce the problem even with patch from upstream: https://github.com/WebKit/WebKit/pull/18271 on bluebox app.

pgorszkowski-igalia commented 10 months ago

It is reported on upstream: https://bugs.webkit.org/show_bug.cgi?id=263521. I created also PR: https://github.com/WebKit/WebKit/pull/19559, when it merges I will backport.

pgorszkowski-igalia commented 9 months ago

The fix is merged in upstream and backported to downstream wpe-2.38 (https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/1233) and wpe-2.42 (https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/1234) so I close this issue.

jakub-gocol-red commented 9 months ago

I confirm. Looks good now. I don't see this leak anymore