Should {future,stream}.close-{readable,writable} take ownership of the error-context?

[alexcrichton]

Currently in the canonical ABI these intrinsics only acces the error-context rather than consuming it, like a borrow<_> instead of own<_>. My (possibly naive) expectation though would be that in the majority of cases that an error context is passed in and then discarded, so should this perhaps both take an error-context and remove it from the component's table?

[dicej]

Keep in mind that error-contexts are neither borrowed nor (exclusively) owned. The handles are reference counted, so you still have access to a handle after you've given it to someone else. You only lose access to a handle once you've called error-context.drop as many times as your local reference count has been incremented.

[alexcrichton]

My current understanding is that your last statement is inaccurate, but let me double-check as I may be wrong:

You only lose access to a handle once you've called error-context.drop as many times as your local reference count has been incremented.

My understanding of CanonicalABI.md is that when you call error-context.drop you instantly lose access to that value in the component. While the host is required to reference count values this isn't semantically visible in the ABI as there's no "clone" operation to incref, right?

---

What I'm wondering is whether it would make sense to remove the error-context given to *.close-* intrinsics from the local component's table as part of the *.close-* intrinsic. That wouldn't match what lifting/lowering do but I also think it would be reasonable to change that to have error-context behave like own<_>. Is there a reason to require reference counting on the host though?

[lukewagner]

Oops, I skimmed the last part of Joel's comment too quickly before 👍 . You're right that there is no reference count: a single call to error-context.drop removes it from a component instances' error-contexts table (and there is never any de-duping or other way in which a single error-context index is given to guest wasm multiple times, so there is no case where a reference count would otherwise be incremented).

[lukewagner]

We could consider a switch to move-semantics, like own. I think there are use cases where you want to deliver a single error-context to multiple destinations, so I think we'd need to add an error-context.dup, though.

[dicej]

Oops, I skimmed the last part of Joel's comment too quickly before 👍 . You're right that there is no reference count: a single call to error-context.drop removes it from a component instances' error-contexts table (and there is never any de-duping or other way in which a single error-context index is given to guest wasm multiple times, so there is no case where a reference count would otherwise be incremented).

OK, I guess I misunderstood, and the implementation in Wasmtime will need to be redone. My understanding was that if I pass an error-context to the host or another component and then get it back, I'll get the same handle back (and become responsible for dropping it twice).

[alexcrichton]

I think there are use cases where you want to deliver a single error-context to multiple destinations, so I think we'd need to add an error-context.dup, though.

Personally I'm a fan of always-own semantics, but I'm naturally pretty heavily biased with Rust.

Thinking about this it's an interesting point I've not fully internalized before, but ownership semantics are more-or-less "no cost" in Rust/C/C++ but are costly in GC'd languages (like JS) since you have to "take" from an object and then have runtime checks to make sure you don't take twice. Conversely borrow-like semantics are "no cost" in GC'd languages (just clean up on a GC of the object) but are costly in Rust/C/C++ as you have to take care to connect the usage of the value to happen before the destructor.

For borrow-semantics it's possible to make that no-cost in Rust/C/C++ so long as something isn't persisted outside of a function call, but once it's persisted beyond a single function call it's (IMO) infeasible to model in C/C++ and requires lifetimes in Rust which may or may not be possible.

All in all, even if I try to set aside my Rust biases (which I know I can't fully do), I feel like own semantics might be the right choice here. Perhaps this is my Rust showing as well but when working with errors I feel like historically most of the time you create an error for a single operation to thread it around and it's much rarer to create an error that gets reused many times.

[lukewagner]

Overall, I think switching to transfer semantics and adding an error-context.dup sounds reasonable b/c it optimizes for what seems like the common case, putting the extra call burden on the "hold onto the error-context" case.

Your comment does bring up the fact that the transfer semantics requires an extra internal mutation in cases where I have a GC object containing an error-context that outlives the lift. E.g., in JS if I throw new Error("foo"), that Error object contains an error-context created by the Error constructor and, without fancy static analysis, the engine has to assume the Error survives until a later GC confirms its unreachable, so any bindings that wanted to extract the error-context from that Error would need to error-context.dup + update the Error. I think this is still perhaps the right tradeoff, but I think it's worth calling out.

[alexcrichton]

Personally I wouldn't be certain whether that tradeoff is right or whether the current non-ownership model is best. It's clear to me how to articulate the pros/cons, but weighing them I'm less certain...

One possible tweak though I'd make is could we remove error-context.dup? For example in JS the error-context object could be delayed to be created right up until it's needed which would avoid the need for duplication.

[lukewagner]

If the idea is that error-context.new (non-deterministically) eventually captures a stack-trace and other host-determined metadata, you want to create it at the new Error() (which is where a JS engine does stack-trace things too). Moreover, if the error-context is being propagated (rather than created anew), then I think you have to treat it like a handle (to an immutable cloneable, but still opaque, resource).

[alexcrichton]

Oh good points! Ok In that case if we go with owning semantics I agree error-context.dup is well-motivated

[lukewagner]

Yeah, I'm ambivalent; even moreso because, since we're on the error path, we're not so concerned with every cycle. @dicej any suggestions?

[dicej]

Yeah, I'm ambivalent; even moreso because, since we're on the error path, we're not so concerned with every cycle. @dicej any suggestions?

I'll admit I've only been half-following this issue, but owning semantics sounds fine with me. If we think we can do without error-context.dup for now (with a possible performance penalty), then let's do that since it's one less thing to implement.

BTW, @vados-cosmonic is currently the expert on the error-context implementation, so he may have opinions. BTW Victor: note that I was wrong about how reference counting should work on a per-component basis, so we'll need to revisit that in wasip3-prototyping.

[vados-cosmonic]

Ah thanks for the heads up @dicej , yeah I've been watching this thread and with regards to the semantics, I don't have a bias really here but I am concerned mostly with least-surprise.

In $LANGUAGE, bindings will have to provide some sort of ErrorContext class (possibly as an inner detail of an existing error class like Error in JS, or not). An incomplete description of the likely operations I imagine will be most common with error-contexts are listed below:

1. Create errors
2. throw/pass created errors
3. Set/modify the message on the error
4. Get one or more causes (which are likely to be nested errors)
5. Get one or more related stack entries
6. Match against some indicator of the error's type (i.e. contents of the error and/or causes, in some way)

(1) and (2) we can do now, and (3) we can do only at construction right now, and the rest (3-5) are all in the future.

In this context, I think that owned semantics w/ transfers + a new error-context.dup sounds reasonable, because mutation seems very likely to be in our future, we could at least avoid having to try reason about mutation without ownership guarantees.

Some more use cases that I think might be worth considering for impact:

• Nesting of ErrorContexts
• Rendering a list of causes and/or stack traces

I think that ref-counted versus owned w/ transfers mostly works the same in this case, but I think the ref-counted approach would require much more intricate/inefficient tracking during implementation of the above?

I'm not sure what we wanted to gain from ref-counting in the first place though, so maybe there's something I'm missing.

A separate thought, but is this a "two way door"? -- can we imagine a world where we add options that facilitate operations on error contexts that are always "borrowed" in every component they're in? Some sort of error-context.new-managed that makes the other use case possible (if it's a good idea to begin with?).

[lukewagner]

One tweak I'd make to the above future-additions list is: while I think we do need to support extending an error-context with additional causes/stacks, I think we need to do this immutably (precisely to avoid the sort of situations that you're describing), where the extension primitives produce new error-contexts based on old ones.

[vados-cosmonic]

Hey @alexcrichton this can probably be closed since we're moving error-context out of these APIs

[alexcrichton]

I've scheduled this to be closed with #489