1279971 - Save Image As... on <canvas> element causes leaking memory in chrome process due to not releasing the data URIs (may cause OOM crash)

Status: RESOLVED WORKSFORME

(Firefox :: Downloads Panel, defect, P3)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: screenshot of about:memory

STR:

1. Find <canvas> elements which can save as images as. (I cannot provide actual website which I realized this bug, sorry)
2. Right click on a <canvas> and do "Same Image As..." and save it.
3. Repeat #2.

Then, when I start to do this, Firefox's parent process uses about 400MB. However, after doing #2 about 200 times, it uses about 2.1GB!

In about:memory, I can see a lot of string objects which have data URI. See the screenshot of it.

GC/CC/Minimize usage memory in about:memory doesn't help this symptom.

I see some times OOM due to this bug:
https://crash-stats.mozilla.com/report/index/a3a12672-c2d2-4af3-80aa-b64c42160614
https://crash-stats.mozilla.com/report/index/2022277e-d2ed-4d37-a5b6-a576c2160613
https://crash-stats.mozilla.com/report/index/6af57218-98e1-4dce-9391-f1a1e2160607

FYI: I can reproduce this bug even in safe mode but I've not tested this on non-e10s mode yet.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I reproduced this even on non-e10s window. So, not e10s specific bug.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Okay, I probably find the holder of the data URIs. That is "Downloads" because I can copy the data URI from context menu on the items and when I use "Clear Downloads" in Library, a lot of memory space is released and the string objects are gone from about:memory.

Changing the components, although, I'm not sure which is right component, "Bookmark & History" or "Downloads Panel".

I think that users *might* want to redo the download from "Downloads". However, it dose not make sense to store big data URIs on memory. I think that just not recording the data URIs are fine.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I tested with this patch: https://hg.mozilla.org/try/rev/d46ca216cc346e6060a7a0eded96b5c143c2d551
As I expect, string objects are grabbed by nsDownload. Therefore, this patch releases the string objects. However, there are still some memory eater. Perhaps, my analysis is wrong, the string objects are not the only cause of this bug because I see this in about:memory:

> 4,095.94 MB (100.0%) -- address-space
> ├──2,075.81 MB (50.68%) -- commit
> │  ├──1,626.01 MB (39.70%) -- private
> │  │  ├──1,607.09 MB (39.24%) ── readwrite(segments=3784)
> │  │  └─────18.92 MB (00.46%) -- (7 tiny)
> │  │        ├───8.13 MB (00.20%) ── readwrite+stack(segments=390)
> │  │        ├───3.94 MB (00.10%) ── execute-read(segments=64)
> │  │        ├───3.81 MB (00.09%) ── readwrite+guard(segments=390)
> │  │        ├───3.01 MB (00.07%) ── readwrite+writecombine(segments=1)
> │  │        ├───0.01 MB (00.00%) ── execute-readwrite(segments=3)
> │  │        ├───0.01 MB (00.00%) ── noaccess(segments=3)
> │  │        └───0.00 MB (00.00%) ── readonly(segments=1)
> │  ├────256.62 MB (06.27%) -- mapped
> │  │    ├──234.19 MB (05.72%) ── readonly(segments=179)
> │  │    └───22.43 MB (00.55%) ++ (2 tiny)
> │  └────193.18 MB (04.72%) -- image
> │       ├──129.83 MB (03.17%) ── execute-read(segments=219)
> │       ├───55.19 MB (01.35%) ── readonly(segments=478)
> │       └────8.16 MB (00.20%) ++ (2 tiny)
> ├──1,370.38 MB (33.46%) ── free(segments=662)
> └────649.75 MB (15.86%) -- reserved
>      ├──628.38 MB (15.34%) ── private(segments=3923)
>      └───21.37 MB (00.52%) ++ (2 tiny)

I think that we need to look for the cause of |1,607.09 MB (39.24%) ── readwrite(segments=3784)|.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I tried a lot like https://hg.mozilla.org/try/rev/7986b48423a3d8c3aad9c5da7d1d2d23a7592f7b but I cannot remove URI from download history and I give up to fix this bug by myself right now.

The patch tries to clear stored URI after finishing the download.  However, I cannot remove it from UI especially from history because URI information are copied a couple of times via some objects and they are not linked.

Additionally, even after clearing download history manually, memory space isn't shrunken enough. For example, From 2GB to 1GB, but if I continue to same things after that, I met OOM crash anyway even if only 1.5GB is used. So, this means that current download code with big data URIs causes serious memory fragmentation due to copying such long string. I guess that it's necessary to redesign download related code. JS objects should refer nsIURI instance instead of string of its spec and shouldn't use URI as a key of hashtable since for creating a key later, objects need to store each big data URI.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Closing because no crashes reported for 12 weeks.