Skip to content

add stream.forward and future.forward builtins (alternative definition) - #717

Open
lukewagner wants to merge 1 commit into
mainfrom
forward
Open

lukewagner wants to merge 1 commit into
mainfrom
forward

Conversation

@lukewagner

Copy link
Copy Markdown
Member

This PR is based on the great work in #709, but reformulates the definition of {stream,future}.forward to avoid the dynamic forward field/indirection on streams/futures that was a bit invasive and complex to think through. Instead the definition in this PR mutates the relevant readable and writable ends in-place which ends up being a lot simpler and, iiuc, closer to what an implementation would actually do. To enable this "just mutate the ends" approach in the second commit, the first commit refactors the definition of streams/futures so that streams/futures always have ends (even when the host is the reader and/or writer). This refactoring was a bit invasive, so it felt bigger than something I could reasonably ask for in a review comment in #709. But it ended up becoming a pretty nice simplification for futures/streams overall that reduced lines of code, removed some special cases, and highlighted a spec bug where DROPPED wasn't delivered as eagerly as it should have been (included as a WAST test in the refactoring commit that Wasmtime passes).

The only actual behavioral difference in this PR vs. #709 that I'm aware of is that there is no trap in the "self-loop" corner case (where the readable and writable ends passed to {stream,future}.forward are both attached to the same stream/future); it's just a no-op (the stream/future is no longer alive since both of its ends were just dropped). But if anyone has any good reasons why it should trap, happy to discuss.

The refactoring/simplification commit is nice, but touches a lot, so if you're only interested in {stream,future}.forward, I'd suggest only looking at the second commit. The second commit also includes a bunch of WAST tests for the various interesting semantic cases.

PTAL @rvolosatovs

@rvolosatovs rvolosatovs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good overall, however it appears that with this refactoring the Python host loses a way to specify a custom host-provided cancel and so "true" async cancellation does not appear to be possible anymore. In other words, it seems like cancel_copy can never return BLOCKED anymore.

I'm not entirely sure whether the Python host implementation in this repo is meant to be "normative" or not, but if it is, then this should probably be addressed

Comment thread design/mvp/canonical-abi/run_tests.py Outdated
Comment thread design/mvp/Concurrency.md
The [Stream State] and [Future State] sections describe the runtime state
maintained for streams and futures by the Canonical ABI.
Given the readable end of one stream/future and the writable end of another, the
`{stream,future}.forward` built-ins can be called to efficiently forward all

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be gated by the forward emoji?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Concurrency.md, since it's high-level and not listing the individual grammar rules or opcodes, I haven't been finely emoji-gating; if you follow the links though you see the emoji-gate on the built-ins.

Comment thread design/mvp/canonical-abi/definitions.py Outdated
Comment thread design/mvp/canonical-abi/definitions.py Outdated
@lukewagner

Copy link
Copy Markdown
Member Author

I split out the refactoring commit into #719 which, incidentally, @rvolosatovs now addresses your comment by explicitly modelling the host blocking cancellation. With #719, the still-rather-short definition of {stream,future}.forward changed again, so it'd be worth taking a second look.

@lukewagner
lukewagner force-pushed the forward branch 2 times, most recently from d53ede8 to d5fe801 Compare September 14, 2026 22:24
Co-authored-by: Roman Volosatovs <rvolosatovs@riseup.net>
@tschneidereit

Copy link
Copy Markdown
Member

AFAICT this approach also doesn't provide a way to forward multiple readables into a single writable one after the other, right? If so, I think that is a substantial limitation, and I'd love to find a way around it. Perhaps the forwarding operation could return a future resolving to the writable (or a result of it), and dropping that future would allow the host to terminate the instance while the operation is in flight?

(I would still prefer richer sequencing support as sketched out here, but can see how that is a nontrivial increase in complexity.)

@lukewagner

lukewagner commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

@tschneidereit That's right; something more complex but, I believe, additive would be needed. I actually really like @rvolosatovs's idea of stream.{split,join} and think they'd address a lot of otherwise-hairy concurrency questions with stream.splice (in any form). But for now, it seems useful to start with just {stream,future}.forward since they're comparatively quite simple.

@tschneidereit

Copy link
Copy Markdown
Member

@lukewagner can you say what wouldn't work about returning a future?

(For full disclosure, I have a real use case that I would very much like to use this for, but which fundamentally requires composing a response out of multiple inputs. I'm concerned about postponing any support for that kind of scenario for what might be a meaningful amount of time. Hence, I'm quite motivated to help make this work however I can 😄)

@lukewagner

Copy link
Copy Markdown
Member Author

@tschneidereit It's not that it couldn't be made to work, it's just that splice is significantly more complex than forward (in particular: unambiguously communicating drop and partial read/write progress correctly in all directions in all cases). Chatting about this topic with @sunfishcode last week, he was recalling the same issues with wasi:io@0.2's splice as well and that maybe stream.{split,join} offered a much cleaner solution. Regardless of whether splice-vs-split/join is the right technical solution, I think it makes sense to work on this incrementally. But if you're motivated to work on this next after forward, I'm happy to as well.

FWIW, yet another alternative option (or just stop-gap solution) we've discussed for these "concatenation of streams" use cases is adding new methods on the relevant WASI interfaces (e.g., WASI HTTP's request and response).

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

Successfully merging this pull request may close these issues.

3 participants