Intent to Implement and Ship: Throw RangeError exception on ArrayBuffer allocation failure

[Justin Novosad]

Contact emails

ju...@chromium.org

Spec

http://ecma-international.org/ecma-262/6.0/#sec-createbytedatablock

Summary

Avoid out-of-memory render process crashes when allocating ArrayBuffers by throwing an exception instead (as specified in ECMAScript 6.0)

Is this feature supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes

Compatibility Risk

For web application that do not trap the exceptions there is no risk: the script context where the error occurred will halt instead of crashing the whole process. For web content that does trap exceptions, there is a small (theoretical?) risk that this particular exception might not be handled correctly which might result in a confused application state.

The motivation for shipping this is that we have a significant bucket of renderer crashes caused by OOM exceptions in CanvasRenderingContext2D.getImageData(). The failures often happen in ads that are isolated in a iframe. By shipping this feature, the ad may halt or fail to render correctly in low memory conditions, without affecting the functionality of the host web page. This is a much better UX than to crash the entire tab. I believe this advantage strongly outweighs the compatibility risk.

OWP launch tracking bug

https://code.google.com/p/chromium/issues/detail?id=536816

Entry on the feature dashboard

None yet. Small change

[Rick Byers]

What do the other browser's do in this situation?

[Jochen Eisinger]

A couple of questions:

- what will you do for the array buffers that V8 uses internally, and without them it can't run (e.g. the one used to store the random seeds). Those are allocated per context, so it can happen that allocation fails during iframe setup.

- what will you do for array buffers created in blink (and only later exposed to v8 if at all). Will you continue to crash there, or will you make sure that all codepaths that allocate array buffers can cope with allocation failures?

[Justin Novosad]

On Mon, Sep 28, 2015 at 11:45 AM, Jochen Eisinger <joc...@chromium.org> wrote:

A couple of questions:

- what will you do for the array buffers that V8 uses internally, and without them it can't run (e.g. the one used to store the random seeds). Those are allocated per context, so it can happen that allocation fails during iframe setup.

- what will you do for array buffers created in blink (and only later exposed to v8 if at all). Will you continue to crash there, or will you make sure that all codepaths that allocate array buffers can cope with allocation failures?

My plan is to throw the DOM exception in all cases where the failed allocation happens inside the scope of a script API call (including constructors), and to leave the current behavior unchanged in all other cases. For special cases like buffers created in blink and exposed to script later, I will leave the current behavior unchanged (crash) and will add TODOs and file bugs so that they can be reviewed on a case-by-case basis. I suppose that in some cases, we will just want to defer the exception.

[Justin Novosad]

On Mon, Sep 28, 2015 at 11:39 AM, Rick Byers <rby...@chromium.org> wrote:

What do the other browser's do in this situation?

According to Boris Zbarsky's reply on the whatwg thread, Gecko throws an exception : https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Sep/0040.html

It is not clear however if this comment only related to the getImageData/createImageData APIs which were the subject of the discussion, or whether it related to all/most forms of ArrayBuffer allocations.

I have no information on other implementations

[Anne van Kesteren]

On Mon, Sep 28, 2015 at 6:10 PM, 'Justin Novosad' via blink-dev
<blin...@chromium.org> wrote:
> My plan is to throw the DOM exception in all cases where the failed
> allocation happens inside the scope of a script API call (including
> constructors), and to leave the current behavior unchanged in all other
> cases.

Could you please file specification bugs along the way? It isn't clear
to me how this would work, e.g., for XMLHttpRequest. It seems there we
might want to catch the exception and return null instead.


--
https://annevankesteren.nl/

[Justin Novosad]

Right, async APIs in general should probably catch the exception and take appropriate action.  Ex: Promise based APIs should reject the Promise rather than propagate the exception.

Does this mean that all affected APIs need to have their specs tweaked? Or is there some catch-all edit that could be made? 

 

--
https://annevankesteren.nl/

[Anne van Kesteren]

On Mon, Sep 28, 2015 at 7:24 PM, Justin Novosad <ju...@google.com> wrote:
> Right, async APIs in general should probably catch the exception and take
> appropriate action. Ex: Promise based APIs should reject the Promise rather
> than propagate the exception.
> Does this mean that all affected APIs need to have their specs tweaked? Or
> is there some catch-all edit that could be made?

Everything needs to be tweaked since nothing (that I know of) expects
that the allocation operation can fail. At the very least
specifications need to have "Rethrow any exceptions." added, but
likely it's a little more involved (as it is for `<canvas>`,
`fetch()`, and `XMLHttpRequest`).


--
https://annevankesteren.nl/

[Jochen Eisinger]

well, any kind of allocation can always fail with a stack exceeded exception...

[Rick Byers]

On Mon, Sep 28, 2015 at 12:15 PM, Justin Novosad <ju...@google.com> wrote:

On Mon, Sep 28, 2015 at 11:39 AM, Rick Byers <rby...@chromium.org> wrote:

What do the other browser's do in this situation?

According to Boris Zbarsky's reply on the whatwg thread, Gecko throws an exception : https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Sep/0040.html

It is not clear however if this comment only related to the getImageData/createImageData APIs which were the subject of the discussion, or whether it related to all/most forms of ArrayBuffer allocations.

Thanks. 

I have no information on other implementations

In general for an intent to ship I think it's important that we know whether we're going first or not as it has a huge impact on the compat risk and the amount of up-front diligence we might want to do (eg. devrel outreach).  This should be trivial to check, right?

But in this case, even if we were going first, the risk seems low enough that I'd be OK just attempting to ship and keeping an eye out for issues.  So LGTM1 to to ship.

You should probably make sure the MDN docs for ArrayBuffer are updated to include this detail (if you create a chromestatus entry for this, updating MDN will probably be done for you, but you can also just edit yourself).

[Justin Novosad]

On Mon, Sep 28, 2015 at 2:22 PM, Rick Byers <rby...@chromium.org> wrote:

This should be trivial to check, right?

I thought so, but not really. I wrote an html test that tries to run the renderer into the ground by calling getImageData repeatedly on a very large canvas and accumulating ImageData objects in an array (so they don't get garbage collected).  Firefox, at least on 64-bit linux, grows for a very long time to fill my swap partition, making my system grind to a halt before the browser itself gets to experience an OOM condidtion. With blink it is not so bad because PartitionAlloc craps out earlier. All this to say: it is quite painful to test this using html. In my experimental blink CL, I am adding instrumentation to generate synthetic allocation failures for testing purposes to avoid having to generate real OOM conditions in tests. But for checking other browsers, all I have is an html test.

[Rick Byers]

Ah, interesting.  Perhaps mobile (where RAM is more constrained and swap is non-existent) is a more interesting scenario anyway?

[Rick Byers]

By the way, I also assumed I could just create ridiculously large ArrayBuffers.  I see this already fails with a RangeError on Chrome, but I got some weird results playing with Edge for just a few seconds (probably wrapping).

[Boris Zbarsky]

On 9/28/15 12:15 PM, 'Justin Novosad' via blink-dev wrote:
> According to Boris Zbarsky's reply on the whatwg thread, Gecko throws an
> exception :
> https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Sep/0040.html
> It is not clear however if this comment only related to the
> getImageData/createImageData APIs which were the subject of the
> discussion, or whether it related to all/most forms of ArrayBuffer
> allocations.

SpiderMonkey (the JS implementation in Gecko) will throw an exception on
OOM for all ArrayBuffer allocations.

What Gecko then does with that exception likely depends on the context.
In the case of getImageData/createImageData we just propagate it right
back out to the script. I would expect that to be the common pattern in
Gecko for APIs that create and return a new arraybuffer.

-Boris

[Boris Zbarsky]

On 9/28/15 1:12 PM, Anne van Kesteren wrote:
> Could you please file specification bugs along the way? It isn't clear
> to me how this would work, e.g., for XMLHttpRequest.

I assume you mean for the .response getter?

What Gecko implements here right now is that failure to do the lazy
allocation of an arraybuffer in that getter will throw an exception to
the script that called the getter.

-Boris

[Anne van Kesteren]

On Mon, Sep 28, 2015 at 7:33 PM, Jochen Eisinger <joc...@chromium.org> wrote:
> well, any kind of allocation can always fail with a stack exceeded
> exception...

My understanding is that stack overflow is not specified as part of
the standard, whereas this is. That's why it's important that
standards creating ArrayBuffer instances deal with that failing.


--
https://annevankesteren.nl/

[Anne van Kesteren]

And the next time the getter is called it would throw again? This
doesn't work well with how the specification is setup today (returning
null would be easier), but we could make that work. Not entirely sure
it's better for developers. They probably do not anticipate a getter
to throw. I wouldn't anyway.


--
https://annevankesteren.nl/

[Boris Zbarsky]

On 9/29/15 2:29 AM, Anne van Kesteren wrote:
> On Tue, Sep 29, 2015 at 8:18 AM, Boris Zbarsky <bzba...@mit.edu> wrote:
>> What Gecko implements here right now is that failure to do the lazy
>> allocation of an arraybuffer in that getter will throw an exception to the
>> script that called the getter.
>
> And the next time the getter is called it would throw again?

Well, if the allocation fails again. If the next time it succeeds, then
we'll return the newly-allocated object, and not throw thereafter.

-Boris

[Anne van Kesteren]

On Tue, Sep 29, 2015 at 8:46 AM, Boris Zbarsky <bzba...@mit.edu> wrote:
> Well, if the allocation fails again. If the next time it succeeds, then
> we'll return the newly-allocated object, and not throw thereafter.

Wouldn't it be better to simply return null and free the disk? Seems
like you would have to write rather complicated code to deal with this
eventuality otherwise.


--
https://annevankesteren.nl/

[Anne van Kesteren]

On Tue, Sep 29, 2015 at 9:21 AM, Anne van Kesteren <ann...@annevk.nl> wrote:
> Wouldn't it be better to simply return null and free the disk? Seems
> like you would have to write rather complicated code to deal with this
> eventuality otherwise.

I filed https://github.com/whatwg/xhr/issues/26 &
https://github.com/whatwg/fetch/issues/134. Probably better to discuss
this there. Earlier I also filed
https://github.com/whatwg/html/issues/197. I'd appreciate if someone
else could cover the remaining features.


--
https://annevankesteren.nl/

[Justin Novosad]

That is no longer true in a tip of tree build due to this recent change: https://codereview.chromium.org/1342853002

[Boris Zbarsky]

I assume you mean "free the internal data buffer"?

And I assume you mean complicated code as an XHR API consumer? I think
for the API consumer this sucks no matter how you look at it: you try to
get the data and you can't...

-Boris

[Philip Jägenstedt]

LGTM2 to sorting this out, which may of course involve spec changes that aren't obvious. I don't think all the changes need to be made together, so starting with the easy ones and moving on seems fine.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Jochen Eisinger]

lgtm3

Please cc me on the CLs