Appropriate error for exceeding an implementation limit for table or memory size

[lars-t-hansen]

At the moment, the Wasm JS API spec uses "[EnforceRange] unsigned long" for a number of parameters to API methods. For values outside the [0..2^31-1] range, a TypeError is thrown on function entry. This differs from our past behavior, where we would throw RangeError, but it's what WebIDL requires.

At the same time, we still throw RangeError if maximum is present and maximum < initial.

The question is what we should do if the initial or max value exceeds the maximum allowable by the implementation. For example, for a Table, the max is 1e7 elements. Firefox has been throwing RangeError here, but to a large extent this is as a side effect of using the same logic for checking for 1e7 as it would use to check for 2^32-1.

As I am updating Firefox to be WebIDL-compliant, the fallout would be that we throw TypeError for when we exceed 1e7 as well, that is, we will effectively assume that the value of the parameter is restricted to [0..1e7]. However I could choose to use TypeError only for the checks against the parameter type, but use RangeError when we exceed the implementation limit.

Does anyone have opinions about this?

[littledan]

Sorry for the churn from the WebIDL transition. I bet we can all agree that the more important thing than these broad error classes is the detailed error messages that implementations give. To defend the current specification, at the risk of being pedantic, I think a TypeError could make sense for things which are out of range with respect to an integer type, whereas RangeError could make sense for the WebAssembly implementation limits and maximum < initial cases. However, I'd be fine with switching them all to TypeError if anyone has a strong preference. I'm not sure making a RangeError version of EnforceRange would pay for itself in terms of WebIDL complexity.

[lars-t-hansen]

I don't really have a strong preference, it's just a detail that we need to agree on :)

Even though I think TypeError is less than ideal for integer values outside the unsigned long range, I think this ship has sailed - let's use WebIDL as it is and not futz with this. Consistency with the rest of the world is worth something.

Since I agree that it would make the most sense to use RangeError for exceeding implementation limits when we use RangeError for maximum < initial, I will vote with my feet and update my patches to throw RangeError for this case.

[littledan]

OK, does this mean nothing needs to be changed for this issue?

[lars-t-hansen]

OK, does this mean nothing needs to be changed for this issue?

Probably. Both Table::grow and Memory::grow specify that a RangeError is thrown if the grow fails, regardless of reason (exceed implementation limit or actual OOM). I happen to think that it's slightly unhelpful to throw RangeError for an out-of-memory condition but I can live with it just fine.

[littledan]

Well, the intention with those two errors were to match JS: JS specifies that creating too big of an ArrayBuffer throws a RangeError. At the same time, implicitly in JS (since the spec doesn't deal with resource limits) and explicitly in Wasm (here), an implementation can do whatever it wants in an out of memory condition (presumably, including some of the time that a RangeError would be thrown). JS engines have non-matching behavior from each other (IIRC some crash, some throw an OOMError or something, certain conditions do actually result in a RangeError; @caitp had some fun working on getting a resilient implementation of this in V8 once), so I'm not sure how useful matching the rest of the OOM behavior will be.

Any ideas for what we should do to the spec in light of this? A really long, boring note?

[lars-t-hansen]

It would be sort of nice to say that if the implementation deals with OOM (usually this means it deals with OOM for large allocations only, or for specific allocation sites only, like here) then it must throw a specific type of exception. This would be esp nice for smaller environments like phones. And that exn can be RangeError, as I wrote; we can probably all live with that. The real danger is diverging implementation behavior.

Given that OOM currently has unspecified effects and that implementations are diverging, it is probably just as well to implicitly leave the exn throw by OOM as RangeError?

If we were going to spec something for OOM I would want to spec it specifically per API method, not as blanket behavior.

[littledan]

It would be sort of nice to say that if the implementation deals with OOM (usually this means it deals with OOM for large allocations only, or for specific allocation sites only, like here) then it must throw a specific type of exception. [...] If we were going to spec something for OOM I would want to spec it specifically per API method, not as blanket behavior.

My intuition is sort of the opposite: If we want to start Doing Standards here, I would imagine making an unconditional, universal call, "All OOMs result in a catch-able (xyz)Error". Being conditional or per-API doesn't sound very useful.

This would be esp nice for smaller environments like phones.

(Don't most smartphones run Chrome or variants of it, which (I think) has OOM-crashing behavior? It's really hard to usefully, reliably recover from OOM; IMO Chrome makes a decent choice here.)

[lars-t-hansen]

It would be sort of nice to say that if the implementation deals with OOM (usually this means it deals with OOM for large allocations only, or for specific allocation sites only, like here) then it must throw a specific type of exception. [...] If we were going to spec something for OOM I would want to spec it specifically per API method, not as blanket behavior.

My intuition is sort of the opposite: If we want to start Doing Standards here, I would imagine making an unconditional, universal call, "All OOMs result in a catch-able (xyz)Error". Being conditional or per-API doesn't sound very useful.

On the contrary. Recovering from OOM for large allocations is frequently possible and desirable, since large allocations not only take up a lot of memory but fail with much higher probability for reasons of fragmentation. Being able to recover leads to a better UX. Recovering from OOM for small allocations is much harder in general when it happens, because then resources really are exhausted, and one must take action to free up resources in an orderly way and immediately if one is to continue running; but small-allocation OOM conditions are also less likely.

This would be esp nice for smaller environments like phones.

(Don't most smartphones run Chrome or variants of it, which (I think) has OOM-crashing behavior? It's really hard to usefully, reliably recover from OOM; IMO Chrome makes a decent choice here.)

Obviously most smartphones /should/ be running Firefox, and with the recent ruling in the EU one can hope that Firefox starts showing up as the default browser on more Android phones... but I digress.

Firefox tries fairly hard not to crash on OOM, but will sometimes do so anyway for known-small allocations, for reasons outlined above. And crash on OOM is becoming a more meaningful choice with process separation and process-per-tab.

But even when the general strategy is crash-on-OOM, it is attractive to try to signal an error for large allocations so that the application can recover. Once we start seeing more wasm content on 32-bit that blithely tries to allocate 1GB memory we will start to worry about this more.

[lars-t-hansen]

To complete that thought: I think that for APIs like grow(), we could somewhat reasonably say that if the implementation fails to acquire enough linear memory to satisfy the request then it should throw RangeError, and then perhaps note that the effect remains unspecified if it fails for other reasons, including failing to allocate auxiliary data, failing to acquire backing store on a lazy-commit system, and so on. But I'd be interested to hear other impementers' takes on this.

[littledan]

I see, thanks for taking the time to explain @lars-t-hansen . Well, the current spec does say RangeError, e.g., in step 6 of WebAssembly.Memory.prototype.grow, so maybe we're all good then? (Maybe the Wasm grow_memory instruction needs to have its error behavior spelled out better?) Or maybe we should add notes to come to a stronger position on those sorts of metaphysical/implicit aspects that I was talking about above?

[lars-t-hansen]

I think that at this point I would add a note simply saying that what happens on OOM is unspecified even for large allocations but that we as a group should try to find some consensus about whether we can spec something better (and moderately testable) for the failed-to-allocate-large-linear-buffer case. Implementer input is definitely needed.

[littledan]

OK, so this could be one of those "Issue X" notes? How about going in the section that talks about OOM errors in general?

[lars-t-hansen]

I don't know what an "Issue X" note is but adding something to the OOM section sounds fine, for now. So long as we don't forget to do something about it in the longer term.

[binji]

Seems like this issue was resolved, closing.