PSA: Throw exception when text encode alloc memory fail.

[xu ms]

Contact emails: xuzh...@microsoft.com

Summary：

We are currently observing many renderer crashes occurring in text encode.Encoding Standard (whatwg.org)

This is because DOMArrayBuffer::Create is currently used to create a buffer, and when memory allocation fails, renderer process crashes. The reasons for memory allocation failure are unclear and not solely due to allocating excessively large memory.

Therefore, we want to change the logic here so that when memory allocation fails, a DOMException is thrown from text encode instead of crashing.

Blink component: Blink>TextEncoding

Tracking bug:[OOM] Is it a real OOM for blink::DOMArrayBuffer::Create? [355018938] - Chromium

[Mike Taylor]

Hi there,

Have we done any sort of web compatibility analysis of what this change implies? A broken page might be a better choice than a crashed tab, but it's hard to know without any sense of the potential impact of this change. Also, is there a plan to specify this behavior? What's the interop situation?

thanks,
Mike

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/f4cfdc62-707f-4d21-80d5-43ed37ce52fan%40chromium.org.

[Adam Rice]

Usually specs don't cover what happens when you run out of memory, as implied by https://infra.spec.whatwg.org/#algorithm-limits. I think this is fine. I'm interested in what other browsers do, but it's hard to test unless you have a VM handy.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/c6b00d44-599f-47bf-be3d-9e977681b827%40chromium.org.

[Yoav Weiss (@Shopify)]

On Fri, Aug 2, 2024 at 11:40 AM Adam Rice <ri...@chromium.org> wrote:

Usually specs don't cover what happens when you run out of memory, as implied by https://infra.spec.whatwg.org/#algorithm-limits. I think this is fine. I'm interested in what other browsers do, but it's hard to test unless you have a VM handy.

This section indicates that algorithms shouldn't impose specific limits, but I don't think that implies that there shouldn't be standard behavior when those limits are (naturally) met. An example of that is the QuotaExceededError exception. We expect developers to be able to deal with different quota levels in different browsers, but the exception they get when they hit the limit is the same everywhere.

IMO, it may make sense to define an exception that developers can expect in those cases. (maybe depending on what other browsers are doing in that case)

On Fri, 2 Aug 2024 at 01:17, Mike Taylor <mike...@chromium.org> wrote:

Hi there,

Have we done any sort of web compatibility analysis of what this change implies? A broken page might be a better choice than a crashed tab, but it's hard to know without any sense of the potential impact of this change. Also, is there a plan to specify this behavior? What's the interop situation?

thanks,
Mike

On 8/1/24 4:38 AM, 'xu ms' via blink-dev wrote:

Contact emails: xuzh...@microsoft.com

Summary：

We are currently observing many renderer crashes occurring in text encode.Encoding Standard (whatwg.org)

This is because DOMArrayBuffer::Create is currently used to create a buffer, and when memory allocation fails, renderer process crashes. The reasons for memory allocation failure are unclear and not solely due to allocating excessively large memory.

Therefore, we want to change the logic here so that when memory allocation fails, a DOMException is thrown from text encode instead of crashing.

Blink component: Blink>TextEncoding

Tracking bug:[OOM] Is it a real OOM for blink::DOMArrayBuffer::Create? [355018938] - Chromium

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/f4cfdc62-707f-4d21-80d5-43ed37ce52fan%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/c6b00d44-599f-47bf-be3d-9e977681b827%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAC_ixdwDfGZQgUNP7HSkU03heC8VG2Zy8fqhJJWzxDerV1i8zA%40mail.gmail.com.

[Dave Tapuska]

If you look at the OOM handler of Windows version in PartitionAlloc we do not debug alias the size of allocation. So you may not see the size that was failing in the minidump.  Have you had any success in getting a histogram of the sizes of the allocations that are failing from those minidumps? Could it be a bug in the TextEncoding that is asking for a very large allocation size?

Also there is CrashMemoryMetricsCollector on Android that you could expose on desktop to collect any additional histograms via shared memory (ie. I don't know if a histogram of the size of allocation that failed might be useful or not).

Perhaps the partition alloc folks have any ideas that additional reasons could be logged in crash keys or in the shared memory crash metrics.

dave.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOmohS%2Bcb1M4gjXTbJ-Hyv3DDpBbxFU6-U4gyZZZnxmOffvqaA%40mail.gmail.com.

[Adam Rice]

Could it be a bug in the TextEncoding that is asking for a very large allocation size?

I've checked the code, and it doesn't appear to have major issues. If the input is latin1, it allocates 3*length, where it only needs 2*length, but that doesn't seem like a major issue.

According to https://chromium-review.googlesource.com/c/chromium/src/+/5742992/comments/03be0cf3_828fe5f2 Firefox and Safari do different things, and this change will make us match Firefox.

[Dave Tapuska]

Does it make sense to put it behind a feature flag? I know an OOM is terminal, I'm just wondering if something could have a side effect of not OOMing and removing some state when the exception fires instead.

dave.