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] Custom conversions between exceptions in wasm versus in JS #190

Closed
RossTate opened this issue Nov 8, 2021 · 17 comments
Closed

Comments

@RossTate
Copy link
Contributor

RossTate commented Nov 8, 2021

By request, this is a fork from #183.

Motivation

The basic premise is to enable wasm modules to specify how they would like JS exceptions to converted into their wasm exceptions at the module's entry points, and similarly how they would like their wasm exceptions to be converted into JS exceptions at the exit points.

One use case is JS interop. For example, Kotlin will want to have a $__kotlin_exception : [(ref $Throwable)] exception. To match existing JS interop, Kotlin will want all JS exceptions to be converted into Kotlin exceptions at the boundary: if the JS value is already a Kotlin Throwable, just make it the payload of the exception, and otherwise wrap the JS value as an externref field of some (internal) JSException subclass of Throwable. Similarly, Kotlin will want Kotlin exceptions that escape to JS to be converted into JS exceptions as follows: if the value is a JSException, extract and throw its externref, and otherwise throw the ref $Throwable as a JS value (using the JS API for GC that lets GC objects be used as JS objects).

Another use case is debugging. The current EH design and JS API only provides good debugging support if you conform to a particular convention. But this convention is too restrictive to conveniently extend this support to features such as C++'s throw;, Go's defer, and Java's finally. With custom conversions, wasm generators can assume all exceptions have a specific tag, and they can explicitly propagate debugging information such as stack traces (as externref values) so that, at the boundary, they can create JS Errors with this debugging information that already hooks into tooling such as debuggers.

Strategy

The high-level strategy is to have "call an Exported Function" in the JS API call a custom-defined wasm function when a wasm exception is thrown, and to have "create a host function" in the JS API call a custom-defined wasm function when a JS exception is thrown. The former function takes the payload of the exception and returns the externref to be thrown in JS. The latter function takes the externref that was thrown and throws the corresponding wasm exception; if the function returns (void), then the externref is considered unhandled and treated as a trap (just as in the pre-EH JS API).

To clarify, this strategy does not make use of a WebAssembly.Exception class, nor is there a special tag with payload [externref] for catching/throwing JS exceptions in wasm with; it simply treats all exceptions from JS uniformly and expects them to be explicitly converted to wasm exceptions at the boundary (if they're to be handled at all).

Also, while I'm using JS as a concrete example, all the designs here work just as well for other embeddings.

Design

Given the high-level strategy, the key design question is how to specify/determine the custom conversion functions to use.

Wasm-to-JS Exception Conversion

A key challenge to keep in mind is that one can create funcrefs from wasm functions even if the functions are not explicitly exported, and these funcrefs can be passed to and called from JS.
So we need to ensure that even the exceptions thrown by these functions are converted, ideally in the same manner as if the function were called as an explicit export on the module.

The design that seems to achieve this most simply is for the module defining an exception tag of type [t*] to associate a wasm function of type [t*] -> [externref]. The engine stores this function in the tag, and the wasm-to-JS stubs generated by engines catch wasm exceptions and call the function stored in the tag with the payload of the exception and throw the returned JS value.

We could make this association optional, in which case the default behavior would be to simply trap.

JS-to-Wasm Exception Conversion

A key challenge to keep in mind is that many applications will want to be able to convert arbitrary exceptions from JS into their wasm exceptions.
Any value can be an exception in JS, so this means we cannot make any assumptions about what structure the JS-exception value has.

With this constraint, I can think of three designs.

Instance-Directed Conversion

The first design is for module's to be able to specify a distinguished [externref] -> [] function, akin to how modules can specify a distinguished start function. When a JS function is used as an import for a wasm instance, that instance's distinguished [externref] -> [] function is used to convert any exceptions it throws. (If none is specified, then the JS exception is propagated as a trap.)

Import-Directed Conversion

The second design offers slightly more control. For each import, a module can specify which function to use to handle JS exceptions.

Type-Directed Conversion

Both of the above have some issues. For one, they both only handle the cases where JS functions are converted to explicit imports. But the js-types proposal allows you to create wasm funcrefs with just a type signature. So they'd require extending that proposal to create a funcref with an exception handler (that would typically be exported from the wasm instance the funcrefs eventually get propagated with). To potentially make matters worse, the purpose of WebAssembly/design#1408 is to let tooling bypass js-types and just use table.set and the like, but the funcrefs created in this manner would also not have any JS exception handler (so they'd just trap).

Another problem is that they only let the exception handler kick in when the import is a JS function. If, on the other hand, the function comes from wasm then no conversion will happen, which will violate the expectations of the program. I understand this isn't a big issue right now because C++ wasm modules are always surrounded by JS glue, but the JS API we're working on for the GC proposal would eliminate the need for glue. Furthermore, ESM integration will make it more difficult for users to anticipate when a wasm module will get hooked up with another JS module or another wasm module. So this will likely become a problem soon.

The approach that addresses both of these issues and nicely mirrors the wasm-to-JS design is to use a type-directed approach. Given a wasm module that type-checks with unchecked exceptions but uses only the $__cpp_exception tag, just by adding throws $__cpp_exception to every function type in the types section (you don't have to touch anything else) you get a wasm module that type-checks using checked exceptions. If you do that, then you can use the exception tag(s) in the throws clause of an import to determine which conversion function to use. To do so, you associate with each exception tag $exn a wasm function of type [externref] -> [] (throws $exn), and the JS-to-wasm stub for a function type that throws $exn uses that conversion function. (If the throws clause specifies multiple exceptions, you try their conversion functions in the sequence listed.)

This works for funcrefs created by WebAssembly/design#1408. It also works for hooking up two wasm instances (via the JS API or ESM): if the imported function throws tags not accepted by the expected function signature, they are converted to externref using their associated conversion-to-JS functions, and that externref is then converted to the expected exception tags using their associated conversion-from-JS functions.

@dschuff
Copy link
Member

dschuff commented Nov 9, 2021

It sounds like you are expecting that this conversion function would need to be called on every transition during unwinding from JS to or from wasm, else it would be considered unhandled and convert to a trap. I don't think that's actually necessary. The conversion only needs to happen when an exception is actually caught (either in JS or in wasm) or passed to an onerror handler, which would be at the end of unwinding rather than in the middle.

Also, given that this would be an addition to the existing API, it's not actually necessary that an unconverted exception gets turned into a trap, but I can see how it might make sense to offer that as an alternative to the MVP behavior.

Does the type system of the GC spec (or the typed funcref spec) include types that may be thrown? I seem to recall that being discussed in the context of interface types, but not elsewhere.

I think for this to be useful for linear-memory languages such as C++, it would be better to make the callback a JS function, rather than a wasm function. Any throw that wants to include metadata such as a stack trace will have to call a JS function to get it, and the final conversion for any unhandled exception will be to JS. (Since the final conversion will need to create a JS object, any wasm callback would just need to call back out to JS to do that). The JS code could of course call any exported wasm function it wanted.

@RossTate
Copy link
Contributor Author

RossTate commented Nov 9, 2021

The conversion only needs to happen when an exception is actually caught (either in JS or in wasm) or passed to an onerror handler, which would be at the end of unwinding rather than in the middle.

Or passed to the debugger (the conversion provides the stack trace). One of these cases seems to always happen. I worked through a number of use cases and implementation strategies and found that almost always the conversion-at-the-boundary approach worked better from many perspectives. The only counterexample I could think of is when the matching catch of a thrown wasm exception is a wasm catch of the same tag with a JS frame in between, in which case there's an unnecessary coercion to JS and coercion back. But this seemed like a very uncommon scenario that wasn't worth optimizing for.

I can go through the list of reasons if you'd like, but it's a long one, so I don't want to go into it unless you really think it's necessary.

Does the type system of the GC spec (or the typed funcref spec) include types that may be thrown?

No. As low-level features, GC is orthogonal to exceptional control throw. We would need to make a new EH design to develop a notion of throwable values (though, I'm not sure this would be useful, since in many languages all values are throwable).

What there has been discussion of is making GC refs be able to be subclasses of JS classes in the JS API. So Kotlin/Java's Throwable could be made a subclass of JS's Error. But it would really be up to the JS API for EH to make it so that a thrown KotlinJava Throwable becomes a thrown JS value when it crosses the boundary, rather than a WebAssembly.Exception with the $__kotlin/java_esception tag and the Throwable value as the payload.

I think for this to be useful for linear-memory languages such as C++, it would be better to make the callback a JS function, rather than a wasm function. Any throw that wants to include metadata such as a stack trace will have to call a JS function to get it, and the final conversion for any unhandled exception will be to JS.

The reason to make it a wasm function is that in many cases it will want to access internal state of the module. For example, to make throw; handle foreign exceptions in C++, libcxxabi will need to have a (very small) externref table of foreign exceptions (mirroring the structure of the exception stack libcxxabi already implements in linear memory). The coercion function will need to check if the exception is "foreign" and, in which case, access this internal table to get the externref (which it simply returns). As another example, to make throw; preserve stack traces, while you could make the stack trace be part of the payload, that'll result in a lot of instructions just for carrying around the stack trace. It'd be more compact and efficient to make use of libcxxabi's exception stack and have a corresponding externref table storing the stack trace of each exception currently being handled/propagated, and again the coercion function will need to access this internal table. (In this case, it will need to import a function to make a JS Error with that stack trace, but that's pretty easy to do.)

I worked through a number of cases and found that having the converter be in wasm rather than JS was always at least as efficient and easy to use, and in some cases much more so.

@aheejin
Copy link
Member

aheejin commented Nov 9, 2021

It mostly sounds like you want to revive exnref, with its name changed to externref. Whatever the name is, many problems you associated with exnref, including 2PEH, will again rematerialize then. Does this mean now you think the removal of exnref, which you insisted for months last year, was not a good idea after all?

@RossTate
Copy link
Contributor Author

Whatever the name is, many problems you associated with exnref, including 2PEH, will again rematerialize then.

Wait, @aheejin, your post suggests a potential critical miscommunication between us. What do you mean by the problems for 2PEH would "rematerialize"? I ask because the change from unwind to catch_all+rethrow already made the current proposal incompatible with 2PEH. (As in, you'll have to design a whole new set of throw/catch/unwind instructions that ignore the ones in this proposal entirely if you want to add 2PEH.) So I've been operating with the understanding that the problems for 2PEH have already materialized through conscious decision, and so I'm trying to get at least 1PEH to work well. (I can articulate why exnref has problems even for 1PEH, but it would be good to first make sure we're on the same page about the current proposal and 2PEH.)

@aheejin
Copy link
Member

aheejin commented Nov 10, 2021

I think I answered the same question from you a while ago: #176 (comment)

To repeat, it's not rethrow and catch_all themselvesare incompatible with 2PEH. The way the toolchain is using them for cleanups is, which we can switch to unwind in a future 2PEH proposal, if one is ever made. It will be a trivial task. Also I think "extensibility" is a better word, and this is the one you used in #118. Having exnref and not having it is a huge fundamental change. If we have exnref now, then it will not be really extensible to a future 2PEH proposal that doesn't have it.

Also this writeup kind of assumes the existence of exnref (or externref; the name is not important) and wasm code can access that reference type in the core spec, which makes this not a JS API-only suggestion, despite its title. It's confusing that you advocated for its removal for months and now want to restore it. I agree it gives us more freedom in the toolchain, which was the reason I pushed back for a long time last year. But that ship has sailed now.

@rossberg
Copy link
Member

But that ship has sailed now.

I keep hoping it has not. :)

@RossTate
Copy link
Contributor Author

Thanks for the clarification (and reminder), @aheejin. It was very useful.

The issue with that is that the value of unwind really comes into play when everyone uses it. Remember that I got involved just after Andreas had presented how EH had been specifically designed as a first piece of a multi-language collaborative control system ("effects"), so my understanding was that the group was aiming to support a multi-language ecosystem like .NET and Racket had both successfully done. But both those systems rely on everyone using unwind-like constructs to facilitate these cross-language interop. Thus having the by-far predominant producer of wasm go out of its way to not use unwind is pretty much a killer of that opportunity. (And, if a producer wants to support 2PEH purely internally, they can do so more flexibly using simpler primitives like what Michal showed Common Lisp uses.)

It's sad in some ways that that opportunity is gone, but a huge advantage of not striving for that is that it's much easier for producers to generate non-collaborative WebAssembly. With the goal of just non-collaborative EH in mind, there are two things to account for:

  1. Optimize the design for internal exceptions, i.e. exceptions thrown and caught by the same instance (or closely related group of instances).
  2. Convert between internal and external exceptions at the boundary.

exnref did (2) a bit, but not particularly well. It was essentially GC-style exceptions, but with no incorporation into GC (e.g. no way for Java/Kotlin's Throwable to both be an exnref and extend Java/Kotlin's Object layout), and consequently not supporting the sort of desired interop in the OP. But more problematically, exnref did not do (1) at all. For the common case of throwing your own exception to your own handler up the stack, it required allocating an exnref and then downcasting it via br_on_exn, even just to get a look at the payload only to determine it's not the right (Java/Kotlin/C++) type and rethrow it. C++ may not care about this overhead because C++ EH is notoriously inefficient and C++ programmers already avoid EH for performance-critical code. But other languages have found that inefficient EH can cause 6x slowdown on programs, so adding all these superfluous casts (which the GC proposal has found to be surprisingly expensive) and allocations is not great. On top of this, exnref made stack tracing entirely implicit, introducing further performance issues and limiting means to get debugging support.

The current proposal has the potential to improve on (1) significantly (maybe even optimally?). A throw could find its matching catch by just searching the stack for the right tag and jumping to the specified handler address. But the inclusion of polymorphic exception handling via catch_all+rethrow means that engines are likely instead boxing exceptions (so that they can uniformly store them for catch_all+rethrow) and downcasting them at each catch, recreating the poor performance characteristics of exnref. But more problematically, the proposal does nothing about (2), leaving a mess for producers trying to provide good interop or maintain an invariant that only one kind of exception propagates through internally. There's also the issue of debugging support, since the current proposal really ties your hands if you want debugging support by keeping it implicit.

So my concern is that the change from unwind to catch_all+rethrow made the proposal into something between 2PEH and 1PEH that serves neither case well. From what I can tell, the simplest way to address that (in the 1PEH direction) is to get the JS API to address (2) while addressing the more advanced JS-interop use cases mentioned in the OP. And, since an improvement to the JS API would also make it easy for producers to explicitly manage debugging information, it would also eliminate the need for catch_all+rethrow, addressing (1) and overall making the proposal easier to generate (though it's your choice whether to actually remove those instructions).

Given that, what are thoughts on the motivations and/or solution strategies in the OP?

@dschuff
Copy link
Member

dschuff commented Nov 10, 2021

But that ship has sailed now.

I keep hoping it has not. :)

We've gotten way off topic from this issue, but we have to finish MVP exceptions.
We can't keep going back and forth on these major issues, and jerking around everyone who wants to implement this in engines and toolchains, and especially everyone who wants to use it for their real apps that already exist today. Unlike, say, GC, which unlocks future langauges (and thereby future applications), this is a critical missing component for languages, developers, applications, and users who are depending on this platform today, and who are critical to its success (and therefore our ability to even keep doing this work at all).
We've done all the hard work (twice) in validating this proposal in the real world and with our guesses as to what will happen in the future, and advanced it 99% of the way to standardization. We can't afford to go back and do it again. If it turns out that we made mistakes, we should work on correcting them when it becomes more clear in retrospect what they actually are.

@aheejin
Copy link
Member

aheejin commented Nov 10, 2021

@RossTate I have to repeat what I said in the previous comment. The OP is based on the implicit assumption that there is a exnref-like reference type that can be accessed in core wasm, and discussing JS API specifics you laid out while glossing over your exnref assumption makes no sense. Also, if the JS API specifics are limited to the JS API (which OP is not), I think it'd better be discussed in a separate follow-on proposal due to its nontrivial design space, as I said in another issue.

Also, br_on_exn's alleged inefficiency is beside the point. We can have exnref with possibly better casting/extracting instructions, but that's not what you argued for last year. As long as we can store exnref in a local and pass it around freely, 2PEH wouldn't work, which is also what you argued for.

I simply cannot agree that the current LLVM toolchain not using unwind is a deal breaker for a future 2PEH proposal. As I said, that switch to unwind will be trivial; I don't think it will even take a few hours to change that. You are picking on an implementation detail of a toolchain and arguing it is a fundamental spec problem. But exnref's existence is a huge difference both in the toolchain and VMs.

It's sad in some ways that that opportunity is gone,

I don't think it's gone.

but a huge advantage of not striving for that is that it's much easier for producers to generate non-collaborative WebAssembly.

I pushed back for months last year arguing just this, but you said the toolchain could work around the restrictions by various workarounds in many posts. Seeing my arguments coming from you after we and the VM people did all the work second time over is a very interesting situation to be in.

@RossTate
Copy link
Contributor Author

We've done all the hard work (twice) in validating this proposal in the real world and with our guesses as to what will happen in the future, and advanced it 99% of the way to standardization.

My understanding is that this is the phase where tooling becomes a concern.

Multiple (1PEH) language teams have been invited to look at this proposal, and every team (that I know of) has articulated reasons why this proposal does not suit their needs well. I have independently investigated some challenges of languages as well and also articulated problems, with no solutions offered.

The C++ compiler does not support stack-trace-preserving throw;, nor does it support catching (or rethrowing) of foreign exceptions due to these issues.

I actually do not know of a 1PEH language team that has been able to successfully compile their language's full EH functionality to this proposal. That does not seem validated.

Seeing my arguments coming from you after we and the VM people did all the work second time over is a very interesting situation to be in.

@aheejin I understand that you are frustrated by my seemingly changing position. After the CG voted to go ahead with a design with unwind, you made a unilateral decision (against the recommendation of the outside expert y'all invited to offer their opinion on this matter) to change it to catch_all+rethrow, and so I am working within that constraint. I did write a post sharing my preliminary thoughts under that constraint long ago in #149.

We can't afford to go back and do it again.

I understand, and my understanding is that this is the phase where the JS API gets developed, and the conversions I am suggesting are executed solely in the JS API.

@conrad-watt
Copy link
Contributor

conrad-watt commented Nov 10, 2021

After the CG voted to go ahead with a design with unwind, you made a unilateral decision (against the recommendation of the outside expert y'all invited to offer their opinion on this matter) to change it to catch_all+rethrow, and so I am working within that constraint

I don't think this is accurate. IIRC the decision to remove unwind was discussed here and there was broad agreement.

@aheejin
Copy link
Member

aheejin commented Nov 10, 2021

The C++ compiler does not support stack-trace-preserving throw;, nor does it support catching (or rethrowing) of foreign exceptions due to these issues.

Again, you are conflating the current LLVM toolchain's implementation status with the spec issue. We compile catch (...) + rethrow foreign exceptions into using catch_all and rethrow. We just haven't done that just because we had other priorities.

you made a unilateral decision (against the recommendation of the outside expert y'all invited to offer their opinion on this matter) to change it to catch_all+rethrow

I am not sure if all that happened in #142 can be characterized as that. In that post, several people expressed concerns (or 👍ed) on why unwind had to exist in the MVP proposal, just to have a different meaning in a future 2PEH proposal. Also #153 had agreements from several stakeholders.

@RossTate
Copy link
Contributor Author

@conrad-watt Sorry, to clarify, the unilateral decision was to add catch_all+rethrow, with the understanding that unwind would be used for unwinding. However, @aheejin never actually used unwind, at which point the vote you pointed to was obvious. One reason I do not invite outside experts to this group anymore is because Michal met with me and expressed how upset he was with how he was treated and misled.

We compile catch (...) + rethrow foreign exceptions into using catch_all and rethrow. We just haven't done that just because we had other priorities.

Please get this working (with throw;).

@RossTate
Copy link
Contributor Author

I closed this issue because do not think it will productive and I do not wish to engage anymore. It started off productive until the champion's first post, which focused on attacking me rather than engaging in the ideas. Things escalated from there (and apologies to the group for partaking in that escalation).

@aheejin
Copy link
Member

aheejin commented Nov 11, 2021

I am sorry if you felt that way; I didn't mean to attack you in my first comment. I was mostly confused that it looked like you wanted to bring back something resembling what you said was problematic in many ways last year.

Also maybe one thing I said in the comments was misleading. I agree this can be JS API-only, if we make every language's tag externref. But that will still have the same problems exnref used to have regarding 2PEH, for the reasons we removed it.

And I don't remember LLVM not using it at the moment played a role in unwind's removal in #153 or any other discussions. and I thought, and still think, catch_all+rethrow can coexist with unwind. It might have been better if you made your objections explicit in #153 and #156, but I wonder whether relitigating this will be worth it at this point. But even if unwind still exists, I think we still need a good JS API for JS-wasm conversion.

@aheejin
Copy link
Member

aheejin commented Nov 11, 2021

Also I am sorry that Michal felt that way. I don't remember all the details of the discussions with him in #142, but I think he was suggesting things including a very different SjLj-like EH scheme for Lisp, in which it takes O(1) from throwing to catching, and I thought at Phase 3 we couldn't afford to change the current proposal in that fundamental way, so I think I argued that a new proposal could be a better way to solve that problem. I still believe that's the case, but I guess I could have been better in listening to him and discussing the issues.

@RossTate
Copy link
Contributor Author

Thank you very much, @aheejin, for those posts. They're really impactful and greatly appreciated, especially since I recognize how frustrating and problematic all this has been from your position as well.

If you want to engage on the topics of the OP, feel free to reopen the issue and prompt a renewed discussion. But I understand if you don't want to.

Cheers.

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

5 participants