bytecodealliance / wrpc

Wasm component-native RPC framework
Other
152 stars 21 forks source link

fix(wit-bindgen-wrpc): clear out variant decoder internal state after use #257

Closed vados-cosmonic closed 2 months ago

vados-cosmonic commented 2 months ago

This commit fixes a bug which manifests itself when decoding a list of enums that may have separate internal decoder objects to use.

The issue is that storing the internal state inside the bindgen'd Decoder becomes a problem on the second (or subsequent) elements of a list of the given enum.

Given a vec![SomeEnum::X, SomeEnum::Y] the saved "state" for the first becomes the first decoder (i.e. the decoder for SomeEnum::X). When it comes time to parse the discriminant and bytes for Y the existence of the state causes a short circuit.

As the (wrong) decoder will generally try to process some bytes, you lose bytes from the input and then bytes are left over at the next attempt to decode (and then an error occurs). How this manifests is obviously specific to the enum, (wrong) decoder, and involved variants, how they are decoded -- i.e. where the decoding will fail.

This only happens for generating bindings for variant types (i.e. Rust enums), which use the PayloadDecoder machinery to dynamically pick which payload decoder to use, depending on the discriminant read.

vados-cosmonic commented 2 months ago

Ah, so that's what the use case there was for -- to try to reuse an existing async decoder? Thanks for taking a look!

rvolosatovs commented 2 months ago

Ah, so that's what the use case there was for -- to try to reuse an existing async decoder? Thanks for taking a look!

the decoders are generally meant to be reused indeed, which is why resetting their state is important once decoding succeeds (as was fixed in your commit)

Tokio interface is not sufficient for our use case, however, since we may also require deferred decoding of an async value after the synchronous part is done, which is what the Deferred trait handles. The deferred function (if any) must be preserved from the decoder state, which is what I've addressed in https://github.com/bytecodealliance/wrpc/pull/257/commits/4314bb29c57c6d296e4c69b8cbe126196332c984