Stream: git-wasmtime

Topic: wasmtime / PR #7713 refactor: allow `Subscribe::ready` to...

Wasmtime GitHub notifications bot (Dec 20 2023 at 17:10):

rvolosatovs opened PR #7713 from rvolosatovs:feat/fallible-ready to bytecodealliance:main:

In the current implementation Subscribe::ready is assumed to be infallible, which does not work in a generic case, where the implementation may require, in fact, to cause a trap in the guest.
This is important, because implementations that require ability to trap in host wasi::io::poll/pollable pollable would otherwise require to implement their own wasi::io::poll/pollable, meaning that they would not be able to reuse all the existing WASI crate implementations (https://docs.rs/wasmtime-wasi/latest/wasmtime_wasi/preview2/trait.Subscribe.html#implementors), potentially locking developers out of most of WASI functionality (due to the [Pollable`](https://docs.rs/wasmtime-wasi/latest/wasmtime_wasi/preview2/struct.Pollable.html) concrete type being used in signatures, which cannot be externally constructed)

Wasmtime GitHub notifications bot (Dec 20 2023 at 17:10):

rvolosatovs requested wasmtime-core-reviewers for a review on PR #7713.

Wasmtime GitHub notifications bot (Dec 20 2023 at 17:10):

rvolosatovs requested alexcrichton for a review on PR #7713.

Wasmtime GitHub notifications bot (Dec 20 2023 at 17:10):

rvolosatovs edited PR #7713:

In the current implementation Subscribe::ready is assumed to be infallible, which does not work in a generic case, where the implementation may require, in fact, to cause a trap in the guest.
This is important, because implementations that require ability to trap in host wasi::io::poll/pollable would otherwise require to implement their own wasi::io::poll/pollable, meaning that they would not be able to reuse all the existing WASI crate implementations (https://docs.rs/wasmtime-wasi/latest/wasmtime_wasi/preview2/trait.Subscribe.html#implementors), potentially locking developers out of most of WASI functionality (due to the Pollable concrete type being used in signatures, which cannot be externally constructed)

Wasmtime GitHub notifications bot (Dec 20 2023 at 17:14):

rvolosatovs updated PR #7713.

Wasmtime GitHub notifications bot (Dec 20 2023 at 17:51):

rvolosatovs updated PR #7713.

Wasmtime GitHub notifications bot (Dec 20 2023 at 17:51):

rvolosatovs has marked PR #7713 as ready for review.

Wasmtime GitHub notifications bot (Dec 21 2023 at 13:36):

rvolosatovs updated PR #7713.

Wasmtime GitHub notifications bot (Jan 02 2024 at 19:01):

alexcrichton commented on PR #7713:

Could you describe a bit more when an embedder might want to return a trap via this method? I agree the current interface doesn't allow it, but one reason we chose to not do this is to avoid a common mistake of performing I/O and returning the error in the ready method instead of buffering up the error to get returned from an accessor elsewhere.

Wasmtime GitHub notifications bot (Jan 08 2024 at 14:03):

rvolosatovs commented on PR #7713:

Could you describe a bit more when an embedder might want to return a trap via this method? I agree the current interface doesn't allow it, but one reason we chose to not do this is to avoid a common mistake of performing I/O and returning the error in the ready method instead of buffering up the error to get returned from an accessor elsewhere.

First I'll comment on this part:

buffering up the error to get returned from an accessor elsewhere

This may not be possible in a generic case, because for this to work the host would need to know which accessor is coupled with the pollable and this relationship is not in any way encoded in WIT. In other words, the host must be aware of the executed Wasm component runtime behavior/logic in order to expose the error in the accessor. Consider wasi:http/types.future-trailers, for example:

resource future-trailers {
  subscribe: func() -> pollable;
  get: func() -> option<result<result<option<trailers>, error-code>>>;
}

Because the host at compile time is aware of the behavior of wasi:http/future-trailers, it knows that if an error was encountered in Subscribe::ready implementation of the returned pollable, wasi:http/future-trailers.get should return it.

But in case of some generic resource, which is only known at runtime, e.g.:

resource custom-trailers {
  subscribe-one: func() -> pollable;
  subscribe-two: func() -> pollable;
  get: func() -> option<result<result<option<trailers>, error-code>>>;
  foo: func() -> string;
}

The host may not in any way know how to communicate the error (and which one) to the guest. The interface, may, in fact, not even utilize an accessor and simply rely on the fact that the pollable has returned "ready". This is precisely the behavior of wasi-clocks timeouts https://github.com/WebAssembly/wasi-clocks/blob/52335b59451193cb36830588298085a76fc78ff1/wit/monotonic-clock.wit#L33-L37.

Coming back to the first part:

My exact use case is a "pollable", which is a remote entity accessible via network:
1. Subscribe::ready establishes the connection to the remote peer
2. Any relevant data is communicated over the connection, an empty response is received by the caller
3. Subscribe::ready unblocks

If network connection in 1. fails, it can just be retried, perhaps with backoff - that part works well with existing API.
If, however, (unrecoverable) protocol error is encountered in 2. - causing a trap in the guest seems to be the only reasonable approach, since the remote entity state is not known and there is no generic way to communicate the error to the guest.

Wasmtime GitHub notifications bot (Jan 08 2024 at 14:04):

rvolosatovs edited a comment on PR #7713:

Could you describe a bit more when an embedder might want to return a trap via this method? I agree the current interface doesn't allow it, but one reason we chose to not do this is to avoid a common mistake of performing I/O and returning the error in the ready method instead of buffering up the error to get returned from an accessor elsewhere.

First I'll comment on this part:

buffering up the error to get returned from an accessor elsewhere

This may not be possible in a generic case, because for this to work the host would need to know which accessor is coupled with the pollable and this relationship is not in any way encoded in WIT. In other words, the host must be aware of the executed Wasm component runtime behavior/logic in order to expose the error in the accessor. Consider wasi:http/types.future-trailers, for example:

resource future-trailers {
  subscribe: func() -> pollable;
  get: func() -> option<result<result<option<trailers>, error-code>>>;
}

Because the host at compile time is aware of the behavior of wasi:http/future-trailers, it knows that if an error was encountered in Subscribe::ready implementation of the returned pollable, wasi:http/future-trailers.get should return it.

But in case of some generic resource, which is only known at runtime, e.g.:

resource custom-trailers {
  subscribe-one: func() -> pollable;
  subscribe-two: func() -> pollable;
  get: func() -> option<result<result<option<trailers>, error-code>>>;
  foo: func() -> string;
}

The host may not in any way know how to communicate the error (and which one) to the guest. The interface, may, in fact, not even utilize an accessor and simply rely on the fact that the pollable has returned "ready". This is precisely the behavior of wasi-clocks timeouts https://github.com/WebAssembly/wasi-clocks/blob/52335b59451193cb36830588298085a76fc78ff1/wit/monotonic-clock.wit#L33-L37.

Coming back to the first part:

My exact use case is a "pollable", which is a remote entity accessible via network:
1. Subscribe::ready establishes the connection to the remote peer
2. Any relevant data is communicated over the connection, an empty response is received by the caller
3. Subscribe::ready unblocks

If network connection in 1. fails, it can just be retried, perhaps with backoff - that part works well with existing API.
If, however, (unrecoverable) protocol error is encountered in 2. - causing a trap in the guest seems to be the only reasonable approach, since the remote entity state is not known and there is no generic way to communicate the error to the guest (and such may not be available even when the interface is known at compile time).

Wasmtime GitHub notifications bot (Jan 08 2024 at 14:07):

rvolosatovs edited a comment on PR #7713:

Could you describe a bit more when an embedder might want to return a trap via this method? I agree the current interface doesn't allow it, but one reason we chose to not do this is to avoid a common mistake of performing I/O and returning the error in the ready method instead of buffering up the error to get returned from an accessor elsewhere.

First I'll comment on this part:

buffering up the error to get returned from an accessor elsewhere

This may not be possible in a generic case, because for this to work the host would need to know which accessor is coupled with the pollable and this relationship is not in any way encoded in WIT. In other words, the host must be aware of the executed Wasm component runtime behavior/logic in order to expose the error in the accessor. Consider wasi:http/types.future-trailers, for example:

resource future-trailers {
  subscribe: func() -> pollable;
  get: func() -> option<result<result<option<trailers>, error-code>>>;
}

Because the host at compile time is aware of the behavior of wasi:http/future-trailers, it knows that if an error was encountered in Subscribe::ready implementation of the returned pollable, wasi:http/future-trailers.get should return it.

But in case of some generic resource, which is only known at runtime, e.g.:

resource custom-trailers {
  subscribe-one: func() -> pollable;
  subscribe-two: func() -> pollable;
  get: func() -> option<result<result<option<trailers>, error-code>>>;
  foo: func() -> string;
}

The host may not in any way know how to communicate the error (and which one) to the guest. The interface, may, in fact, not even utilize an accessor and simply rely on the fact that the pollable has returned "ready". This is precisely the behavior of wasi-clocks timeouts https://github.com/WebAssembly/wasi-clocks/blob/52335b59451193cb36830588298085a76fc78ff1/wit/monotonic-clock.wit#L33-L37.

Coming back to the first part:

My exact use case is a "pollable", which is a remote entity accessible via network:
1. Subscribe::ready establishes the connection to the remote peer
2. Any relevant data is communicated over the connection, an empty response is received by the caller
3. Subscribe::ready unblocks

If network connection in 1. fails, it can just be retried, perhaps with backoff - that part works well with existing API.
If, however, (unrecoverable) protocol error is encountered in 2. - causing a trap in the guest seems to be the only reasonable approach, since the remote entity state is not known and there is no generic way to communicate the error to the guest (and such may not be available even when the interface is known at compile time). Returning in this case could break assumptions of the Wasm component logic and blocking forever would stall execution forever (e.g. if no timeout is used in the Wasm component poll)

Wasmtime GitHub notifications bot (Jan 08 2024 at 16:35):

alexcrichton commented on PR #7713:

Ok makes sense, thanks for explaining. Can this be fixed though by updating the WIT-level APIs?

For example in your custom-trailers I would say that what subscribe and the pollables mean depends on how the WIT APIs are defined. It would be an API contract that subscribe-one would indicate readiness of get and subscribe-two would indicate readiness of foo where foo would have to semantically define what it means to be called before it's ready, etc.

In your use case of a remote-owned resource, this is something that I'd at least personally expect to go through an error somewhere in the WIT. One option would be that when the pollable says "ready" any future operations on it trap (communicating the fatal error) or by updating the APIs there to surface an error instead.

Last updated: Dec 23 2024 at 12:05 UTC