-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Throw exception when text encode alloc memory fail. #334
Comments
Crashing the website process does actually seem preferable over having to define for each API what OOM means. There's currently no standards effort that I'm aware of around standardizing OOM, but if anything it should probably be discussed within TC39 to start with. |
In practice browsers have already abandoned consistent behaviour here, as some OOM failures cause more pain for users than others. XHR is particularly terrible in Blink, as it just silently replaces the response with an empty response in case of OOM. In this specific case, there's no danger of leaving the user agent in an inconsistent state, as we have to do the allocation before anything else. However, user JavaScript may still be in an inconsistent state. In general, the exception will stop it continuing with erroneous calculations, but if it happens to be catching and discarding exceptions when I recognize the wisdom of the oom-fails-fast proposal, but it doesn't reflect what engines actually do. I've had to fix security holes in Blink caused by stack overflow throwing an exception in places that the standard says should never throw. My philosophy is that OOM should continue to crash in most cases, but we need to pragmatically carve out exceptions in cases that cause particular user pain. As such, I'd like to attempt convergence between browsers on those exceptions, and one way to do that is to specifically mention them in the relevant standards. |
In the current encode method, DOMArrayBuffer::Create is used to create a DOMUint8Array. When creation fails, it directly triggers an OOM crash. There are a large number of crashes here, so we want to mitigate them by using DOMUint8Array::CreateOrNull and throwing an exception when creation fails. Bug: 355018938 Change-Id: I3976ff1b330b5611ded8f8cbfadfef93f2d39e37 Blink-Chat: https://groups.google.com/a/chromium.org/g/blink-dev/c/duKL2xRvIaw/m/0yhaqk5EBAAJ Encoding-Github: whatwg/encoding#334 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5742992 Reviewed-by: Marijn Kruisselbrink <[email protected]> Commit-Queue: xu ms <[email protected]> Reviewed-by: Leon Han <[email protected]> Reviewed-by: Adam Rice <[email protected]> Cr-Commit-Position: refs/heads/main@{#1340854}
What is the issue with the Encoding Standard?
In Edge and Chrome, we have found that TextEncoder Encoding Standard (whatwg.org) may encounter OOM (Out of Memory) errors when encode text, not only due to large memory allocation but also for some unknown reasons.
Our current strategy is to crash the renderer process. We now want to optimize this behavior by throwing an exception when memory allocation fails.
I tested encode with a long string in Firefox and Safari browsers.
Firefox will throw a exception "out of memory".
Safari will return a empty UInt8Array.
I noticed that the current standards do not define how to handle OOM (Out of Memory) situations.Do we need to establish such a standard?
The text was updated successfully, but these errors were encountered: