Skip to content
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

[js-api] Should rethrow/br_on_exn trap on undefined? #109

Closed
aheejin opened this issue Apr 23, 2020 · 88 comments
Closed

[js-api] Should rethrow/br_on_exn trap on undefined? #109

aheejin opened this issue Apr 23, 2020 · 88 comments

Comments

@aheejin
Copy link
Member

aheejin commented Apr 23, 2020

We decided to make rethrow and br_on_exn trap on a null value in #90. Should we do the same when a given exnref is an undefined value thrown from JavaScript?

Raised by @backes in https://bugs.chromium.org/p/v8/issues/detail?id=10128#c12.

@RossTate
Copy link
Contributor

I think wasm programs should be able to expect that they can rethrow any exnref that was caught by a try-catch.

@RossTate
Copy link
Contributor

Oh shoot, apparently that's already not the case. From what I can tell of the JS API in #86, which lines up with the linked Chromium issue, if JavaScript throws null then the exnref caught by a surrounding try-catch in wasm will be ref.null.

@aheejin
Copy link
Member Author

aheejin commented Apr 23, 2020

This is not about null but about undefined I think? And #86 does not seem to say anything about neither null nor undefined case. (And #86 was written before we decided on #93, so it needs revision too, but it's not relevant with this issue)

@RossTate
Copy link
Contributor

I was trying to figure out what the interaction of wasm exception handling and JS throws was in more detail so that I could understand the problem more, and #86 was the most recent description I could find. In particular, it says that a JS value being thrown is converted to a wasm value via ToWebAssemblyValue. I then noticed that ToWebAssemblyValue converts a JS null to ref.null, hence the follow-up comment above. (It looks like #93 doesn't change that observation.)

So I don't know what to do about my first comment. It seems like a useful principle to always be able to assume you can rethrow an exception you caught. But I didn't realize at the time that the principle is incompatible with the existing design, so I'm at a loss for suggestions. My original thinking was that the semantics of rethrow and br_on_exn should be independent of what JS value was thrown, but it already depends on whether the JS value was null or not, so I'm not sure what general principle to apply. Sorry for my thinking out loud.

@RossTate
Copy link
Contributor

Hmm, I realized that the JS-interop strategy here has the ability to turn any wasm reference into an exnref. First, give the wasm reference to JS code so that it goes through ToJSValue, then throw the resulting reference in JS code. When this exception enters a wasm stack frame, it will go through ToWebAssemblyValue, which is nearly the inverse of ToJSValue. Then wasm code can catch that value as an exnref.

To make this concrete, the following module instantiated with the subsequent JS importObj uses this trick to turn a funcref into an exnref. It then uses a similar trick to turn that exnref back into a funcref so that it can be called. Thus the following is a very roundabout way to call $foo with the argument 5.

(module
    (import "js" "throw_funcref" (func $throw_funcref (param funcref)))
    (import "js" "exnref_to_funcref" (func $exnref_to_funcref (param exnref) (result funcref)))
    (func $call_indirect_i32 (param funcref i32) ...) ;; uses a funcref table and call_indirect to call the given funcref with the given i32 as its argument
    (func $foo (param i32) ...)
    (func $main
        (try
            (call $throw_funcref (ref.func $foo))
        catch
            (call $exnref_to_funcref)
            (i32.const 5)
            (call $call_indirect_i32)
        )
    )
    (start $main)
)
var importObj = {js: {
    throw_funcref: (ref) => throw ref,
    exnref_to_funcref: (ref) => return ref // intended to trap when ref is not funcref
}};

I'm guessing this behavior was not intended? In particular, it seems to imply that is_exnref would have to always return true if it were ever added.

@backes
Copy link
Member

backes commented Apr 24, 2020

Since catch is specified to catch any host exception, and JS can throw any reference, we have to assume that exnref could hold arbitrary host references.
What we need to clarify here is what Wasm can do with these references. I agree with Ross that Wasm should generally be able to rethrow them. If it's a wasm exception, then of course br_on_exn also needs to work. Apart from that, we should probably handle them as opaque references (meaning they don't trap if you br_on_exn or rethrow them).

null seems to be the only special case we need to think about, since this is a value that can be produced from Wasm.

I don't really see how this discussion relates to function references though. Ross' example assumes that funcref can be passed to the host (JS in this case), which means that ToJSValue provides a host reference to the respective funcref, and ToWebAssemblyValue converts back to funcref. This is independent of exception handling, and should be discussed in the reference types proposal.

@RossTate
Copy link
Contributor

The use of function references in my example is just to make things concrete (and its JS API is in the reference types proposal, which I don't think anyone wants to change). The point of my example is that, with the current JS API for exceptions, an exnref can be an arbitrary wasm reference, not just host reference.

The issue I'm pointing out, the issue raised with "undefined", and the existing issue with null are all related because they all have to do with how wasm/JS values are coerced as the cross the wasm/JS boundaries.

@RossTate
Copy link
Contributor

It occurs to me that when C++ programs throw an exception in wasm, they have to specify an event mark, e.g. __cpp_exception, and the exnref is the event mark paired with the payload. Similarly, the above issues would all be addressed by having JS exceptions be some (abstract, universal) event mark, e.g. __js_exception, paired with a (possibly null) JS-reference payload.

@aheejin
Copy link
Member Author

aheejin commented Apr 24, 2020

@backes According to ToWebAssemblyValue and ToJSValue in reference types JS API spec, when funcref goes from wasm to JS, it becomes an ExportedFunction, and when an ExportedFunction goes from JS to wasm, it becomes funcref.

Then when a JS function throws an ExportedFunction object (which might have been originally funcref and became ExportedFunction via ToJSValue), does it become funcref again? But it should be an exnref to be caught. Is that exnref that has funcref in it? Or is that exnref that has some JS object (ExportedFunction) in it?

@rossberg
Copy link
Member

I think some of the recent comments are based on domain errors. An exnref is not the value caught, it contains the value caught! Throwing null from JS still materialises as a non-null exnref when caught in Wasm. That contains some abstract host value, which happens to be the JS null value (though there is no way to observe that). That is unrelated to null exnrefs, which are merely uninitialised exnrefs.

Similarly if you throw an exported function.

ToJSValue and inverse do not even enter the picture.

@RossTate
Copy link
Contributor

Ah, so it sounds like you had in mind something in line with what I was suggesting with making throwing JS exceptions analogous to throwing C++ exceptions.

But I think it's oversimplifying to say ToJSValue and ToWebAssemblyValue do not enter the picture. One has to consider what should happen if a caught-in-wasm wasm-thrown exception exits to JS as an exnref or anyref and then is thrown by JS code, and similarly if the exception was JS-thrown. From the intuition you suggest, if JS code throws a value v, wasm catches it as an exnref, then the exnref gets passed back to JS and JS throws that value e, then e and v should be distinct values. In particular, e should be an exnref-wrapping of v. On the other hand, if wasm catches the JS-thrown value v and then rethrows it, and then JS code catches that as e, then e and v should be the same value. So this means ToJSValue (or the concept thereof) works differently on values exiting wasm through normal means versus values exiting by being thrown, since in the former case exnrefs are left untouched whereas in the latter case they are unwrapped. That's not necessarily a problem; I am just pointing out that wasm-to-JS and JS-to-wasm conversions are very much part of the picture.

@rossberg
Copy link
Member

What you're saying boils down to the question what ToJSValue does for exnref values passed back to JS normally. If that were to coerce exnref values to their contained value then that would be an explicit unwrapping that could not be reverted by ToWAValue. That is, passing the value back to JS would not give you an equivalent exnref. That's probably undesirable.

From the intuition you suggest, if JS code throws a value v, wasm catches it as an exnref, then the exnref gets passed back to JS and JS throws that value e, then e and v should be distinct values.

Fair point, but I think separable. We could still define special behaviour when catching an exnref thrown from JS (because JS itself cannot create such values). Whether that's necessary I'm not sure.

A better alternative might be to provide a special throw method in the JS API for throwing a proper Wasm exception.

In practice, I don't see much of a use case for passing exnrefs to JS, and I'm not sure it should be encouraged.

@RossTate
Copy link
Contributor

If that were to coerce exnref values to their contained value then that would be an explicit unwrapping that could not be reverted by ToWAValue.

Oh, to clarify. I meant to say that specifically JS-values-wrapped-as-exnrefs should be unwrapped when thrown from wasm to JS, not arbitrary exnrefs.

That is, passing the value back to JS would not give you an equivalent exnref.

Even give the above clarification, this point of yours suggests that exnref should not be equatable.

In practice, I don't see much of a use case for passing exnrefs to JS, and I'm not sure it should be encouraged.

Nonetheless, it is possible to do and so needs to be specified. And, being a web standard, we cannot really change this specification later on. This means we have to try to anticipate all of the ways it could be practical now and design for those patterns.

@aheejin
Copy link
Member Author

aheejin commented Apr 28, 2020

Thank you for the clarification. So let me check a few more points:

  • When exnref is rethrown to JS, it is unwrapped, but it is passed to JS by a function argument, it stays as is, right? So I guess we need ToJSValue for exnref I guess, because even if it is not a desired usage, it is possible. This will likely to be a new object in JS. Does it make sense?
  • What happens exnref that is not created by JS is rethrown to JS? Does it get unwrapped or stays that way?

I guess these are the points we should state in our JS API.

@rossberg
Copy link
Member

@aheejin, conceptually, I would say that an exnref is unwrapped at the moment it's rethrown in Wasm (which is why that instruction traps on null), regardless of where it's caught. If caught in Wasm, a new exnref is produced at the catch site. If caught in JS, you either get the JS value contained or some opaque object representing the Wasm exception.

We could define that throwing a Wasm exception on the JS side behaves like a rethrow in Wasm.

Yes, the JS API certainly needs to define something. It'll probably need to define a new form of Wasm exception exotic objects to represent Wasm exceptions on the JS side.

@aheejin
Copy link
Member Author

aheejin commented Apr 28, 2020

Thanks. So I guess we need to define two things:

  1. "some opaque object representing the Wasm exception" (as @rossberg said), which is the type JS gets when an exception thrown within wasm is caught by JS
  2. ToJSValue(exnref), which is the type we get when we pass exnref as an argument to an imported JS function

@backes, what's the current implementation doing on these two things?

@backes
Copy link
Member

backes commented Apr 28, 2020

In V8, Wasm exceptions are currently instances of WebAssembly.RuntimeError with the Wasm payload attached via a private symbol property. This is invisible to JS, but allows us to read back the payload from Wasm (in br_on_exn). Traps are marked as uncatchable via another private symbol.

No special wrapping happens when this object crosses the JS<>Wasm boundary. I.e. JS just catches the WebAssembly.RuntimeError object but is unable to access the private properties. If it rethrows, we still catch it in wasm as if it never crossed the boundary. The same happens if the exnref is passed as an argument to JS, or back to Wasm. We just pass the object itself.

@rossberg
Copy link
Member

@backes, okay, that sounds like what I would expect. Maybe modelling this private property in the spec does not even require a new exotic object, just a [[...]] property on an error object.

@backes
Copy link
Member

backes commented Apr 28, 2020

What is still missing thought is the wrapping when catching a JS-thrown exception in wasm., and unwrapping when rethrowing to JS.
Otherwise we cannot distinguish a null exnref from a caught null object from JS.

That needs to be spec'ed and implemented.

Implementation-wide, an exnref then either holds a proper wasm exception, or a wrapper for an opaque JS value (which could be null, or undefined, or a WebAssembly.RuntimeError that is not a wasm exception). That guarantees that rethrow after a catch never throws (currently it would, if we catch the null objects from JS).

@RossTate
Copy link
Contributor

Suppose $jsreturn and $jsthrow are imported JS functions that respectively just return or throw the passed-in reference. Then here are some things it seems like you'd expect:

  1. $jsreturn always returns the (observably) same reference it was provided.
  2. try {...} catch {call $jsthrow} (in wasm) is equivalent to try {...} catch {rethrow;} (in wasm).
  3. A JS catch will catch the same (modulo identity?) value that was thrown by a JS throw if all intermediate wasm catches rethrow the exnref they caught.

I'm having a hard time making all these work out simultaneously though.

@rossberg
Copy link
Member

@RossTate, under the special case discussed above, (3) would not hold in the case that the value thrown is actually a Wasm exnref. Other than that, what problem do you see?

@RossTate
Copy link
Contributor

RossTate commented Apr 28, 2020

Well y'all haven't specified what happens when JS throws a wasm-exnref-that-is-wrapping-a-JS-thrown-JS-value.

From your description above, it sounds like when wasm catches that value it will be an exnref wrapping a JS value that is an exnref wrapping a JS value. This breaks item 2 in my list (supposing the ... in the try throws a JS value in JS code).

@rossberg
Copy link
Member

Nothing special, same as if throwing any other exnref. It's a subcase of the case I mentioned one up. Assuming the special-case semantics, throwing an exnref in JS would unwrap it, just like rethrow in Wasm.

The other possibility is that we don't special-case throwing an exnref in JS. Then Wasm catch would just produce another exnref (unobservably) containing the first one, i.e., forego (2) as you say. That's the simpler and more regular semantics, but not sure if it wouldn't be surprising/inconvenient for JS folks.

@RossTate
Copy link
Contributor

Assuming the special-case semantics, throwing an exnref in JS would unwrap it, just like rethrow in Wasm.

This then violates item 3. If the value thrown by JS is a wasm-exnref-that-is-wrapping-a-JS-thrown-JS-value, then the value caught by JS will be the wrapped value rather than the exnref. That is, (v) => try {throw v;} catch(e) {return e;} would not be equivalent to the identity function in JS anymore.

@rossberg
Copy link
Member

Yes, isn't that what I said above?

@RossTate
Copy link
Contributor

It wasn't completely clear to me what you were saying. (No fault on your part, just subtle topic.)

Would JS people be alright with making (v) => try {throw v;} catch(e) {return e;} not be equivalent to (v) => return v;? That doesn't seem like a decision for WebAssembly to make, but y'all know the JS community better than me.

@rossberg
Copy link
Member

I'm not sure how much the JS community would care in general -- JS is full of semantic hacks like that -- but it being induced by an API is more questionable, I agree (although that is not unheard of either, the DOM induces a number of behaviours that are not proper JS).

That said, this isn't necessarily implied. The way such a semantics should probably work is by special-casing the Wasm catch when what it catches is an exnref object thrown from JS. Your two snippets would still be equivalent. The exception could even pass through Wasm code unchanged. Only when Wasm catches (and possibly rethrows) it you'd observe special behaviour.

That is, your condition (3) would still be violated, but not the more specific example you're asking about above.

@RossTate
Copy link
Contributor

RossTate commented May 4, 2020

Ah, so your suggestion is to make it so that try {body} catch {rethrow;} is not the same as just body in wasm itself, which seems to make wasm's semantics ugly instead.

It seems like no matter what we do, the semantics is ugly. But, as you mentioned above, there's not really a use case for giving exnrefs to JS. So what if we instead just made ToJSValue fail in some way if the value is an exnref? After all, exnrefs are just an encapsulation of some intermediate exception-handling state, and the complications above suggest that it really just doesn't make sense to hand off this intermediate state to JS, which has no corresponding analog.

It also means that, if exnref is not a subtype of anyref, then what V8 is doing now works totally fine except that it needs to turn a null thrown by JS into some JS_null_exnref value that it converts back to null in JS catch clauses. But there's no need to do this with other JS values since they can't be exnrefs and so br_on_exnref can just never branch on them. And if exnref is a subtype of anyref, then V8 can turn off this optimization and wrap all JS values as JS_exnref values and convert those back when caught in JS code.

@rossberg
Copy link
Member

rossberg commented May 5, 2020

Ah, so your suggestion is to make it so that try {body} catch {rethrow;} is not the same as just body in wasm itself, which seems to make wasm's semantics ugly instead.

Only in a JS embedding. This would be part of the JS API specification, not core Wasm. Which doesn't say it's nice, but JS is full of discontinuities like that.

So what if we instead just made ToJSValue fail in some way if the value is an exnref?

The simplest design would be to simply not do anything special for exnref in JS. Then you simply cannot rethrow a Wasm exception from JS (unless we introduce an API function for that, as I suggested earlier). I'm not sure what ruling out exnrefs at the boundary would buy on top of that.

@RossTate
Copy link
Contributor

RossTate commented May 5, 2020

Just to clarify, by nothing special you mean it gets wrapped just like every other JS value when caught by wasm?

@aheejin
Copy link
Member Author

aheejin commented Jun 8, 2020

I'm confused. I'm mostly confused about what people think of exnref and exnpackage. Some people seems to think they are the same thing while other don't. (I don't, at least so far)

If I understand correctly (please correct me if not), exnpackage is an implicit entity created by wasm throw. Wasm catch wraps it with exnref and returns it. Wasm rethrow implicitly unwraps that exnref and rethrows that internal exnpackage. This is inline with what @rossberg said in #109 (comment). I'll ask questions based on this understanding. This understanding might be mistaken, so please let me know if so.

@RossTate
I'm not sure what you use to represent exnref in wasm side. catch pushes an exnref on stack.

My sense was the following, using @backes's nice notation:

  • ToJSValue(exnpackage) == WA.ExceptionReference(exnpackage)
    

Is this not

ToJSValue(exnref(exnpackage)) == WA.ExceptionReference(exnpackage)

?

ToWebAssemblyValue(WA.ExceptionReference(exnpackage)) == exnpackage

Ditto; shouldn't this be exnref(exnpackage) on the right hand side?

  • a wasm catch of a JS throw of a jsref catches an exnpackage(jsref) unless jsref == WA.RuntimeException(exnpackage), in which case it catches exnpackage

Here again, isn't what wasm catch returns exnref?


@backes
In #109 (comment), do you consider exnref and exnpackage two different things?

@backes
Copy link
Member

backes commented Jun 8, 2020

Thanks for the explanations, @RossTate and @rossberg. So the basic concern is that any kind of "unwrapping" in ToJSValue or ToWAValue would break reference equality. That totally makes sense. I understand that we are OK with giving up on that when throwing/catching between host and wasm, but not when passing parameters and returning references.
And the proposed solution is to use WA.ExceptionReference for function calls/returns (i.e. in ToJSValue and ToWAValue), and WA.RuntimeException for throw/catch, correct?

Then I still don't see how that solves the issue. @RossTate proposed to still unwrap it in ToWAValue (in #109 (comment)):

My sense was the following, using @backes's nice notation:

  • ToJSValue(exnpackage) == WA.ExceptionReference(exnpackage)
    ToWebAssemblyValue(WA.ExceptionReference(exnpackage)) == exnpackage
    

But then a wasm function taking an exnref parameter and returning it again would break reference equality expectations when called from JS (since we would unwrap when going to wasm, and re-wrap when going back to JS).

The only solution I see to this is to keep a reference to the wrapper when unwrapping, such that re-wrapping creates the same wrapper reference.

@backes
Copy link
Member

backes commented Jun 8, 2020

@backes
In #109 (comment), do you consider exnref and exnpackage two different things?

For me, exnref is a type and exnpackage is a value of that type.

@rossberg
Copy link
Member

rossberg commented Jun 8, 2020

@backes:

But then a wasm function taking an exnref parameter and returning it again would break reference equality expectations when called from JS (since we would unwrap when going to wasm, and re-wrap when going back to JS).

The trick is that (1) this conversion happens regardless of which type you're looking at, and (2) it is a 1-to-1 mapping, i.e., when returning the same exnpackage multiple times you ought to see the same ExceptionRef on the JS side -- similar to how it works for functions. The easiest way to implement that obviously is to use the same representation for exnpackage and ExceptionRef(exnpackage), i.e., the conversions become identity function in the implementation. Alternatively, have each object point to the other.

@backes
Copy link
Member

backes commented Jun 8, 2020

@backes:

But then a wasm function taking an exnref parameter and returning it again would break reference equality expectations when called from JS (since we would unwrap when going to wasm, and re-wrap when going back to JS).

The trick is that (1) this conversion happens regardless of which type you're looking at, and (2) it is a 1-to-1 mapping, i.e., when returning the same exnpackage multiple times you ought to see the same ExceptionRef on the JS side -- similar to how it works for functions. The easiest way to implement that obviously is to use the same representation for exnpackage and ExceptionRef(exnpackage), i.e., the conversions become identity function in the implementation. Alternatively, have each object point to the other.

Oh, so (2) is indeed the "keeping a reference to the wrapper" trick I mentioned. The easiest way to keep a reference of course is to use the same representation. Neat!

If we do the conversion regardless of the type (exnref vs externref (previously anyref)), this means that within wasm exnpackage(X) must compare equal to X, right? I.e. if we create a wasm function taking an exnref and an externref, and we pass the same reference from JS, they must compare equal in wasm, independent of the JS value passed.
But then how do we solve the "JS null does not translate to wasm nullref" constraint?

@rossberg
Copy link
Member

rossberg commented Jun 8, 2020

If we do the conversion regardless of the type (exnref vs externref (previously anyref)), this means that within wasm exnpackage(X) must compare equal to X, right?

No, ExceptionRef(exnpackage(X)) is in a 1-to-1 correlation with the exnpackage(X) object, not with X. When you pass the former to Wasm, you get the exnpackage, not X. Inside Wasm, you cannot ever observe X directly. If it is a Wasm exception, you can access its arguments using br_on_exn. If X is a JS value then you cannot observe it in Wasm. The only way to observe it is by rethrowing and catching it in JS.

So:

ToJSValue(exnpackage(X)) = ExceptionRef(exnpackage(X))
ToWAValue(ExceptionRef(exnpackage(X))) = exnpackage(X)

WACatch(WAThrow(exn, wasmval*)) = new exnpackage(exn, wasmval*)
WACatch(WARethrow(exnpackage(X)) = new exnpackage(X)
WACatch(JSThrow(jsval)) = new exnpackage(jsval)

JSCatch(JSThrow(jsval)) = jsval
JSCatch(WAThrow(exn, wasmval*)) = WA.RuntimeException(exn, wasmval*)
JSCatch(WARethrow(exnpackage(exn, wasmval*))) = WA.RuntimeException(exn, wasmval*)
JSCatch(WARethrow(exnpackage(jsval))) = jsval

All these are agnostic to whether jsval is null or an ExceptionRef. Edit: Specifically,

ToWAValue(WA.RuntimeException(X)) = externref(WA.RuntimeException(X))
ToJSValue(externref(WA.RuntimeException(X))) = WA.RuntimeException(X)

WACatch(JSThrow(WA.RuntimeException(X))) = exnpackage(WA.RuntimeException(X))

unlike the special rule for ExceptionRef. That is, exnrefs are opaque in JS (as ExceptionRef objects), whereas RuntimeExceptions are opaque in Wasm (as externrefs).

@rossberg
Copy link
Member

rossberg commented Jun 8, 2020

Now I wonder again why we can't unify WA.RuntimeException and ExceptionRef. I need a break to get the knot out of my brain.

@rossberg
Copy link
Member

rossberg commented Jun 8, 2020

Yes, so the other alternative I mentioned above would be to collapse WA.RuntimeException and ExceptionRef on the JS side. Then:

ToJSValue(exnpackage(X)) = WA.RuntimeException(exnpackage(X))
ToWAValue(WA.RuntimeException(exnpackage(X))) = exnpackage(X)

WACatch(WAThrow(exn, wasmval*)) = new exnpackage(exn, wasmval*)
WACatch(WARethrow(exnpackage(X)) = new exnpackage(X)
WACatch(JSThrow(WA.RuntimeException(exnpackage(X)))) = new exnpackage(X)
WACatch(JSThrow(other jsval)) = new exnpackage(jsval)

JSCatch(JSThrow(jsval)) = jsval
JSCatch(WAThrow(exn, wasmval*)) = WA.RuntimeException(exnpackage(exn, wasmval*))
JSCatch(WARethrow(exnpackage(exn, wasmval*))) = WA.RuntimeException(exnpackage(exn, wasmval*))
JSCatch(WARethrow(exnpackage(jsval))) = jsval

that is, WA.RuntimeException represents an exnref, and throwing it behaves like a rethrow. However, unlike when throwing normal JS values,

JSCatch(WARethrow(WasmCatch(JSThrow(X))))

gives you a new object when X was a WA.RuntimeException (because Wasm produces a new exnref to be wrapped), but the original X otherwise. That may be confusing.

@aheejin
Copy link
Member Author

aheejin commented Jun 8, 2020

@backes

In #109 (comment), do you consider exnref and exnpackage two different things?

For me, exnref is a type and exnpackage is a value of that type.

This is confusing... We have two different types ExceptionRefernce and RuntimeException in JS, and I thought wasm's exnref corresponds to JS's ExceptionReference and wasm's exnpackage corresponds to JS's RuntimeException... No? IN this case exnref and exnpackage should both be types, even though exnpackage can be an internal one.

(I haven't caught up until the end of the thread yet, so I'm currently assuming there are two separate types ExceptionReference and RuntimeException)

  • ToJSValue(exnpackage) == WA.ExceptionReference(exnpackage)
    ToWebAssemblyValue(WA.ExceptionReference(exnpackage)) == exnpackage
    

But then a wasm function taking an exnref parameter and returning it again would break reference equality expectations when called from JS (since we would unwrap when going to wasm, and re-wrap when going back to JS).

The only solution I see to this is to keep a reference to the wrapper when unwrapping, such that re-wrapping creates the same wrapper reference.

Not sure what this part means. Not sure what happened to exnref in wasm, but anyway in this equation, why would this break reference equality expectations?

@aheejin
Copy link
Member Author

aheejin commented Jun 8, 2020

I think we need clear definitions of each of

  • exnref (wasm)
  • exnpackage (wasm)
  • WA.RuntimeException (JS)
  • WA.ExceptionReference (JS)

at this point. This is really confusing now. Someone uses exnref and exnpackage to be the same thing and someone does not, and it's not clear which in wasm corresponds to which in JS.

@backes
Copy link
Member

backes commented Jun 8, 2020

Yes, so the other alternative I mentioned above would be to collapse WA.RuntimeException and ExceptionRef on the JS side. Then:

ToJSValue(exnpackage(X)) = WA.RuntimeException(exnpackage(X))
ToWAValue(WA.RuntimeException(exnpackage(X))) = exnpackage(X)

WACatch(WAThrow(exn, wasmval*)) = new exnpackage(exn, wasmval*)
WACatch(WARethrow(exnpackage(X)) = new exnpackage(X)
WACatch(JSThrow(WA.RuntimeException(exnpackage(X)))) = new exnpackage(X)
WACatch(JSThrow(other jsval)) = new exnpackage(jsval)

JSCatch(JSThrow(jsval)) = jsval
JSCatch(WAThrow(exn, wasmval*)) = WA.RuntimeException(exnpackage(exn, wasmval*))
JSCatch(WARethrow(exnpackage(exn, wasmval*))) = WA.RuntimeException(exnpackage(exn, wasmval*))
JSCatch(WARethrow(exnpackage(jsval))) = jsval

that is, WA.RuntimeException represents an exnref, and throwing it behaves like a rethrow. However, unlike when throwing normal JS values,

JSCatch(WARethrow(WasmCatch(JSThrow(X))))

gives you a new object when X was a WA.RuntimeException (because Wasm produces a new exnref to be wrapped), but the original X otherwise. That may be confusing.

I like this a lot!

Can you remind me why in these two cases, we need a new exnpackage:

WACatch(WARethrow(exnpackage(X)) = new exnpackage(X)
WACatch(JSThrow(WA.RuntimeException(exnpackage(X)))) = new exnpackage(X)

If we change them to

WACatch(WARethrow(exnpackage(X)) = exnpackage(X)
WACatch(JSThrow(WA.RuntimeException(exnpackage(X)))) = exnpackage(X)

wouldn't that fix your JSCatch(WARethrow(WasmCatch(JSThrow(X)))) for X instanceof WA.RuntimeException case (assuming a 1:1 relation between WA.RuntimeException and exnpackage)? What would it break?

@RossTate
Copy link
Contributor

RossTate commented Jun 8, 2020

None of the explanation I gave was related to object identity. That’s another issue on top of the issues I discussed. Watching kids so can’t help more until later.

@aheejin
Copy link
Member Author

aheejin commented Jun 8, 2020

@rossberg

If we do the conversion regardless of the type (exnref vs externref (previously anyref)), this means that within wasm exnpackage(X) must compare equal to X, right?

No, ExceptionRef(exnpackage(X)) is in a 1-to-1 correlation with the exnpackage(X) object, not with X. When you pass the former to Wasm, you get the exnpackage, not X. Inside Wasm, you cannot ever observe X directly. If it is a Wasm exception, you can access its arguments using br_on_exn. If X is a JS value then you cannot observe it in Wasm. The only way to observe it is by rethrowing and catching it in JS.

So:

ToJSValue(exnpackage(X)) = ExceptionRef(exnpackage(X))
ToWAValue(ExceptionRef(exnpackage(X))) = exnpackage(X)

WACatch(WAThrow(exn, wasmval*)) = new exnpackage(exn, wasmval*)
WACatch(WARethrow(exnpackage(X)) = new exnpackage(X)
WACatch(JSThrow(jsval)) = new exnpackage(jsval)

JSCatch(JSThrow(jsval)) = jsval
JSCatch(WAThrow(exn, wasmval*)) = WA.RuntimeException(exn, wasmval*)
JSCatch(WARethrow(exnpackage(exn, wasmval*))) = WA.RuntimeException(exn, wasmval*)
JSCatch(WARethrow(exnpackage(jsval))) = jsval

All these are agnostic to whether jsval is null or an ExceptionRef. Edit: Specifically,

ToWAValue(WA.RuntimeException(X)) = externref(WA.RuntimeException(X))
ToJSValue(externref(WA.RuntimeException(X))) = WA.RuntimeException(X)

WACatch(JSThrow(WA.RuntimeException(X))) = exnpackage(WA.RuntimeException(X))

unlike the special rule for ExceptionRef. That is, exnrefs are opaque in JS (as ExceptionRef objects), whereas RuntimeExceptions are opaque in Wasm (as externrefs).

I'm totally at loss at this point.

  • What is ExceptionRef here? You said in [js-api] Should rethrow/br_on_exn trap on undefined? #109 (comment) that exnref is created by wrapping by wasm catch and unwrapped by wasm rethrow. Then why would passing exnpackage to JS side create ExceptionRef?
  • What is exnpackage?
  • What is the relationship between exnref and ExceptionRef? It sounds like exnref (wasm) and ExceptionRef (JS) are corresponding equals in each side, but what you said is exnpackage is the counterpart of ExceptionRef instead. Why?
  • Why is WACatch's return value is not exnref but exnpackage?

I'm not sure if terms are defined and people are using the same terms for same meaning at this point.

@backes
Copy link
Member

backes commented Jun 8, 2020

None of the explanation I gave was related to object identity. That’s another issue on top of the issues I discussed. Watching kids so can’t help more until later.

Hm, waiting for more explanation here then. I thought your main concerns are the invariants posted in #109 (comment):

Suppose $jsreturn and $jsthrow are imported JS functions that respectively just return or throw the passed-in reference. Then here are some things it seems like you'd expect:

  1. $jsreturn always returns the (observably) same reference it was provided.
  2. try {...} catch {call $jsthrow} (in wasm) is equivalent to try {...} catch {rethrow;} (in wasm).
  3. A JS catch will catch the same (modulo identity?) value that was thrown by a JS throw if all intermediate wasm catches rethrow the exnref they caught.

I'm having a hard time making all these work out simultaneously though.

The latest proposal from @rossberg (#109 (comment)) with the modification proposed by me in #109 (comment) should satisfy all three points.

@backes
Copy link
Member

backes commented Jun 8, 2020

@aheejin

  • exnref is a wasm type.
  • exnpackage (or "exception package") is a wasm value of type exnref. It's created by a wasm throw, in which case it holds wasm values and the corresponding exception tag (event). It's also created when catching a host exception. How exactly an exception package is implemented is not defined. In web engines, it could directly be backed by a WebAssembly.RuntimeException, but that's only one option to do it.
  • WebAssembly.RuntimeException is the JS type for caught wasm exceptions, potentially also used when passing exceptions to the host as parameters / return values.
  • WebAssembly.ExceptionReference is an alternative proposal for the JS type you get when passing wasm exceptions as parameter / return value. It would behave slightly different to WebAssembly.RuntimeException.
  • The latter two (if we choose to separate them) are only observed in JS. They don't have any meaning in core wasm.

@rossberg
Copy link
Member

rossberg commented Jun 8, 2020

@backes:

Can you remind me why in these two cases, we need a new exnpackage

Because the Wasm semantics should avoid the hacky JS semantics of mutating an existing object, e.g., to attach a different stack trace when caught and rethrown. So each Wasm catch must observably produce a new, immutable exnref object.

  • exnpackage (or "exception package") is a wasm value of type exnref. It's created by a wasm throw, in which case it holds wasm values and the corresponding exception tag (event). It's also created when catching a host exception. How exactly an exception package is implemented is not defined.

I would rather say that an exnpackage is created each time an exception is caught in Wasm, regardless of what kind. Or, in the second variant, also when a Wasm exception is caught in JS.

@aheejin
Copy link
Member Author

aheejin commented Jun 8, 2020

@rossberg

I would rather say that an exnpackage is created each time an exception is caught in Wasm, regardless of what kind. Or, in the second variant, also when a Wasm exception is caught in JS.

Doesn't catch create exnref?

@backes
Copy link
Member

backes commented Jun 8, 2020

@backes:

Can you remind me why in these two cases, we need a new exnpackage

Because the Wasm semantics should avoid the hacky JS semantics of mutating an existing object, e.g., to attach a different stack trace when caught and rethrown. So each Wasm catch must observably produce a new, immutable exnref object.

Ok, understood. I guess I don't care too much about this. We are talking about the JS API here, so inheriting "hacky JS semantics" does not really worry me :)
EDIT: Just realize this is not true. This would be core wasm semantics. Still not fully convinced that this is the best semantics.

  • exnpackage (or "exception package") is a wasm value of type exnref. It's created by a wasm throw, in which case it holds wasm values and the corresponding exception tag (event). It's also created when catching a host exception. How exactly an exception package is implemented is not defined.

I would rather say that an exnpackage is created each time an exception is caught in Wasm, regardless of what kind. Or, in the second variant, also when a Wasm exception is caught in JS.

Agreed, if a wasm catch creates a new exnpackage anyway. Otherwise, I would have said that the exnpackage is created exactly once (on wasm throw), so we can more easily maintain identity. Instead of WA.RuntimeException(exn, wasmval*), we would then have WA.RuntimeException(exn, exnpackage(wasmval*)).

@rossberg
Copy link
Member

rossberg commented Jun 8, 2020

@aheejin:

Doesn't catch create exnref?

Yes, it creates an exnpackage value, which are the values of type exnref.

@backes:

Still not fully convinced that this is the best semantics.

The JS hack is just that, a hack. There should not be any mutation behind your back. It particularly doesn't make sense as long as exnrefs aren't linear. For example, that creates pathological behaviour when you rethrow an exn twice, the second time after you have already caught the first:

try ... catch (e1) {
  try rethrow e1 catch (e2) {
    try rethrow e1 catch (e3) {
      // what's the stack trace of e2 at this point?
    }
  }
}

And this would likely become way more problematic if we wanted to introduce resumption.

@aheejin
Copy link
Member Author

aheejin commented Jun 8, 2020

I'm not sure why we introduced the concept of exnpackage? @backes said exnpackage is a value and exnref is a type, but in JS side we don't have two of them separately. RuntimeException is a type but we also use it as a value too. exnpackage was introduced in the middle of this discussion thread and was not defined until just now.

@backes
Copy link
Member

backes commented Jun 8, 2020

@backes:

Still not fully convinced that this is the best semantics.

The JS hack is just that, a hack. There should not be any mutation behind your back. It particularly doesn't make sense as long as exnrefs aren't linear. For example, that creates pathological behaviour when you rethrow an exn twice, the second time after you have already caught the first: [...]

Well, it's not completely "behind your back" since stack trace modification only happens on a (re)throw. And there is still quite some "magic" happening behind the scenes anyway.
I am not sure, but stack traces would be fully unspec'ed anyway, right? And there is no way to inspect them from wasm directly. So in the end, it is just a JS problem, and JS is already weird. Since it's totally up to the engine whether and how to provide stack traces, I don't see why we need to worry about this here.

Is it correct that with the modification proposed in #109 (comment) (i.e. preserving exnpackage identity across throw/catch/rethrow), we would fully implement @RossTate's wish list?

If so, it's a "we don't want to copy JS hacks (in our JS API)" vs "we want to preserve exnpackage identity" tradeoff. Both has value, but I would consider the latter more useful.

@rossberg
Copy link
Member

rossberg commented Jun 8, 2020

@backes:

I am not sure, but stack traces would be fully unspec'ed anyway, right? And there is no way to inspect them from wasm directly. So in the end, it is just a JS problem

Well, for now, but I wouldn't assume that it will always stay that way. And more likely, stack traces may occur in some API. So I would be cautious to spec something for JS interop that isn't consistent with a well-behaved internal semantics.

@RossTate
Copy link
Contributor

RossTate commented Jun 8, 2020

@backes raised some important issues that have implications beyond the scope of exception handling. In particular, what to do about object identity, which is actually relevant to the following

If we do the conversion regardless of the type (exnref vs externref (previously anyref))

and to the question of whether exnref can be immutable or not. Sorry to be cryptic, but this conversation is already complicated enough, and trying to break down cross-cutting issues here is going to make that much much worse. I'll work on writing up the higher-level topic elsewhere, but in the meanwhile it might be best to pause this conversation for a bit.

@RossTate
Copy link
Contributor

Thanks for pausing this. It took me a while to figure out how to boil down the cross-cutting concerns to digestible discussion topics, but I believe the community's answers to WebAssembly/design#1351 and WebAssembly/design#1353 will help inform the design decisions to make here.

@aheejin
Copy link
Member Author

aheejin commented Oct 12, 2020

Closing this, as we decided to remove exnref in Sep 15 CG meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants