Stream: git-wasmtime

Topic: wasmtime / PR #10044 async/stream/future support for wasm...

Wasmtime GitHub notifications bot (Jan 17 2025 at 22:49):

dicej opened PR #10044 from dicej:wasmtime-wit-bindgen-async to bytecodealliance:main:

I've split this out of #9582 to make review easier.

This patch adds async/stream/future/error-context support to the host binding generator, along with placeholder type and function definitions in the wasmtime crate which the generated bindings can refer to. See https://github.com/dicej/rfcs/blob/component-async/accepted/component-model-async.md#componentbindgen-updates for the design and rationale.

Note that I've added temporary [patch.crates-io] overrides in Cargo.toml until https://github.com/bytecodealliance/wit-bindgen/pull/1130 and https://github.com/bytecodealliance/wasm-tools/pull/1978 have been released.

Also note that we emit a T: 'static bound for AsContextMut<Data = T> when generating bindings with concurrent_imports: true. This is only because rustc insists that the closure we're passing to
LinkerInstance::func_wrap_concurrent captures the lifetime of T despite my best efforts to convince it otherwise. Alex and I suspect this is a limitation in the compiler, and I asked about it on the rust-lang Zulip, but we haven't been able to determine a workaround so far.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Jan 17 2025 at 22:50):

dicej requested alexcrichton for a review on PR #10044.

Wasmtime GitHub notifications bot (Jan 17 2025 at 23:00):

dicej updated PR #10044.

Wasmtime GitHub notifications bot (Jan 21 2025 at 10:53):

rvolosatovs commented on PR #10044:

Also note that we emit a T: 'static bound for AsContextMut<Data = T> when generating bindings with concurrent_imports: true. This is only because rustc insists that the closure we're passing to
LinkerInstance::func_wrap_concurrent captures the lifetime of T despite my best efforts to convince it otherwise. Alex and I suspect this is a limitation in the compiler, and I asked about it on the rust-lang Zulip, but we haven't been able to determine a workaround so far.

In case it helps:

Since async is involved, any chance the error looks anything like https://github.com/rust-lang/rust/issues/63033 ?
If so, a pretty simple fix I have found looks as follows:

• https://github.com/bytecodealliance/wrpc/blob/200518efc47aaa13f8397fc80bdd891d7382f143/crates/transport/src/lib.rs#L44-L51
• https://github.com/bytecodealliance/wrpc/blob/200518efc47aaa13f8397fc80bdd891d7382f143/crates/transport/src/serve.rs#L190-L205

That's the same workaround used in rustc internally: https://github.com/rust-lang/rust/blob/ebbe63891f1fae21734cb97f2f863b08b1d44bf8/compiler/rustc_data_structures/src/captures.rs

Another async bug I've encountered, which appears when Send bound is involved is https://github.com/rust-lang/rust/issues/96865, it sometimes manifests as lifetime issues, but looks like the following when minimized: https://docs.rs/send-future/latest/send_future/trait.SendFuture.html
The fix is straightforward: https://github.com/rvolosatovs/send-future/blob/5637a5982b16b1938680ae9ecac659bdad28b5a7/src/lib.rs#L53-L60

Wasmtime GitHub notifications bot (Jan 21 2025 at 10:57):

rvolosatovs edited a comment on PR #10044:

Also note that we emit a T: 'static bound for AsContextMut<Data = T> when generating bindings with concurrent_imports: true. This is only because rustc insists that the closure we're passing to
LinkerInstance::func_wrap_concurrent captures the lifetime of T despite my best efforts to convince it otherwise. Alex and I suspect this is a limitation in the compiler, and I asked about it on the rust-lang Zulip, but we haven't been able to determine a workaround so far.

In case it helps:

Since async is involved, any chance the error looks anything like https://github.com/rust-lang/rust/issues/63033 ?
If so, a pretty simple fix I have found looks as follows:

• https://github.com/bytecodealliance/wrpc/blob/200518efc47aaa13f8397fc80bdd891d7382f143/crates/transport/src/lib.rs#L44-L51
• https://github.com/bytecodealliance/wrpc/blob/200518efc47aaa13f8397fc80bdd891d7382f143/crates/transport/src/serve.rs#L190-L205
• in bindgen: https://github.com/bytecodealliance/wrpc/blob/200518efc47aaa13f8397fc80bdd891d7382f143/crates/wit-bindgen-rust/src/interface.rs#L172

That's the same workaround used in rustc internally: https://github.com/rust-lang/rust/blob/ebbe63891f1fae21734cb97f2f863b08b1d44bf8/compiler/rustc_data_structures/src/captures.rs

Another async bug I've encountered, which appears when Send bound is involved is https://github.com/rust-lang/rust/issues/96865, it sometimes manifests as lifetime issues, but looks like the following when minimized: https://docs.rs/send-future/latest/send_future/trait.SendFuture.html
The fix is straightforward: https://github.com/rvolosatovs/send-future/blob/5637a5982b16b1938680ae9ecac659bdad28b5a7/src/lib.rs#L53-L60

Wasmtime GitHub notifications bot (Jan 21 2025 at 22:04):

dicej commented on PR #10044:

Thanks for the links, @rvolosatovs. The compiler error I'm seeing doesn't match the ones in those issues, so I suspect it's something different. Here's a (somewhat) minimal example of what I'm seeing:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=cd04abfec0133f3a4c0ed7ab53b04d4b

Wasmtime GitHub notifications bot (Jan 21 2025 at 23:41):

alexcrichton created PR review comment:

Could this be made an optional dependency activated by component-model-async?

Wasmtime GitHub notifications bot (Jan 21 2025 at 23:41):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 21 2025 at 23:41):

alexcrichton created PR review comment:

TIL this is a neat idiom for lack of if let foo = bar && some_other_cond { ... }

Wasmtime GitHub notifications bot (Jan 22 2025 at 00:47):

dicej updated PR #10044.

Wasmtime GitHub notifications bot (Jan 22 2025 at 15:38):

dicej updated PR #10044.

Wasmtime GitHub notifications bot (Jan 22 2025 at 15:57):

dicej updated PR #10044.

Wasmtime GitHub notifications bot (Jan 22 2025 at 16:52):

dicej has marked PR #10044 as ready for review.

Wasmtime GitHub notifications bot (Jan 22 2025 at 16:52):

dicej requested pchickey for a review on PR #10044.

Wasmtime GitHub notifications bot (Jan 22 2025 at 16:52):

dicej requested wasmtime-core-reviewers for a review on PR #10044.

Wasmtime GitHub notifications bot (Jan 22 2025 at 16:52):

dicej requested wasmtime-default-reviewers for a review on PR #10044.

Wasmtime GitHub notifications bot (Jan 22 2025 at 17:04):

dicej updated PR #10044.

Wasmtime GitHub notifications bot (Jan 22 2025 at 17:04):

dicej has enabled auto merge for PR #10044.

Wasmtime GitHub notifications bot (Jan 22 2025 at 17:41):

dicej merged PR #10044.

Last updated: Jan 24 2025 at 00:11 UTC