Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WASI preview 2 output-streams: new backpressure and flushing design #6877

Merged
merged 14 commits into from
Sep 12, 2023

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Aug 21, 2023

Backpressure design is being revised to take into account that any call with a list<bytes> argument is going to consume the resources of the callee, so the callee has to explicitly permit how much that is allowed to consume.

Flushing is a way to request & observe the completion of a write. Since the write itself is non-blocking it cant report that it failed if that happens after the call is over. the explicit flush call allows the caller to wait for a write (or sequence of writes) to produce an error, or guarantee that it succeeded.

Fixes #6811

This PR changes the wit interfaces, therefore we are leaving it as a draft until after WasmCon & Componentize the World (Sept 6-8) so that wasmtime's main reflects a consistent set of interfaces for demos and hacking at those events.

We plan to merge this PR to main on Monday, Sept 11. We will then backport it as a cherry-pick to the 13.0 release branch so that these changes get included in the wasmtime 13 release as well.

@github-actions github-actions bot added the wasi Issues pertaining to WASI label Aug 22, 2023
@elliottt elliottt force-pushed the pch/backpressure_2 branch 2 times, most recently from a63e5c5 to 4fdb4c5 Compare August 25, 2023 07:39
crates/wasi-preview1-component-adapter/src/lib.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/filesystem.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/filesystem.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/filesystem.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/filesystem.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/tcp.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/pipe.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/pipe.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/pipe.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/pipe.rs Outdated Show resolved Hide resolved
@elliottt elliottt force-pushed the pch/backpressure_2 branch 2 times, most recently from 7d19d4b to bffc071 Compare August 31, 2023 21:43
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Sep 1, 2023
* Use `command::add_to_linker` in tests to reduce the number of times
  all the `add_to_linker` are listed.
* Add all `wasi:sockets` interfaces currently implemented to both the
  sync and async `command` functions (this enables all the interfaces in
  the CLI for example).
* Use `tokio::net::TcpStream::try_io` whenever I/O is performed on a
  socket, ensuring that readable/writable flags are set/cleared
  appropriately (otherwise once readable a socket is infinitely readable).
* Add a `with_ambient_tokio_runtime` helper function to use when
  creating a `tokio::net::TcpStream` since otherwise it panics due to a
  lack of active runtime in a synchronous context.
* Add `WouldBlock` handling to return a 0-length read.
* Add an `--inherit-network` CLI flag to enable basic usage of sockets
  in the CLI.

This will conflict a small amount with bytecodealliance#6877 but should be easy to
resolve, and otherwise this targets different usability points/issues
than that PR.
github-merge-queue bot pushed a commit that referenced this pull request Sep 1, 2023
* Use `command::add_to_linker` in tests to reduce the number of times
  all the `add_to_linker` are listed.
* Add all `wasi:sockets` interfaces currently implemented to both the
  sync and async `command` functions (this enables all the interfaces in
  the CLI for example).
* Use `tokio::net::TcpStream::try_io` whenever I/O is performed on a
  socket, ensuring that readable/writable flags are set/cleared
  appropriately (otherwise once readable a socket is infinitely readable).
* Add a `with_ambient_tokio_runtime` helper function to use when
  creating a `tokio::net::TcpStream` since otherwise it panics due to a
  lack of active runtime in a synchronous context.
* Add `WouldBlock` handling to return a 0-length read.
* Add an `--inherit-network` CLI flag to enable basic usage of sockets
  in the CLI.

This will conflict a small amount with #6877 but should be easy to
resolve, and otherwise this targets different usability points/issues
than that PR.
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

This all looks great to me, thanks again so much for pushing on this @pchickey and @elliottt!

I've got a number of minor comments but everything looks great 👍

crates/wasi/src/preview2/filesystem.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/host/io.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/pipe.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/stdio.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/stream.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/tcp.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/tcp.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/tcp.rs Outdated Show resolved Hide resolved
crates/wasi/src/preview2/tcp.rs Outdated Show resolved Hide resolved
pchickey pushed a commit that referenced this pull request Sep 1, 2023
…6950)

* Enhance `async` configuration of `bindgen!` macro (#6942)

This commit takes a leaf out of `wiggle`'s book to enable bindings
generation for async host functions where only some host functions are
async instead of all of them. This enhances the `async` key with a few
more options:

    async: {
        except_imports: ["foo"],
        only_imports: ["bar"],
    }

This is beyond what `wiggle` supports where either an allow-list or
deny-list can be specified (although only one can be specified). This
can be useful if either the list of sync imports or the list of async
imports is small.

* cranelift-interpreter: Fix SIMD shifts and rotates (#6939)

* cranelift-interpreter: Fix SIMD `ishl`/`{s,u}`shr

* fuzzgen: Enable a few more ops

* cranelift: Fix tests for {u,s}shr

* fuzzgen: Change pattern matching arms for shifts

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

---------

Co-authored-by: Jamey Sharp <jsharp@fastly.com>

* Partially revert CLI argument changes from #6737 (#6944)

* Partially revert CLI argument changes from #6737

This commit is a partial revert of #6737. That change was reverted
in #6830 for the 12.0.0 release of Wasmtime and otherwise it's currently
slated to get released with the 13.0.0 release of Wasmtime. Discussion
at today's Wasmtime meeting concluded that it's best to couple this
change with #6925 as a single release rather than spread out across
multiple releases. This commit is thus the revert of #6737, although
it's a partial revert in that I've kept many of the new tests added to
showcase the differences before/after when the change lands.

This means that Wasmtime 13.0.0 will exhibit the same CLI behavior as
12.0.0 and all prior releases. The 14.0.0 release will have both a new
CLI and new argument passing semantics. I'll revert this revert (aka
re-land #6737) once the 13.0.0 release branch is created and `main`
becomes 14.0.0.

* Update release notes

* riscv64: Use `PCRelLo12I` relocation on Loads (#6938)

* riscv64: Use `PCRelLo12I` relocation on Loads

* riscv64: Strenghten pattern matching when emitting Load's

* riscv64: Clarify some of the load address logic

* riscv64: Even stronger matching

* Update Rust in CI to 1.72.0, clarify Wasmtime's MSRV (#6900)

* Update Rust in CI to 1.72.0

* Update CI, tooling, and docs for MSRV

This commit codifies an MSRV policy for Wasmtime at "stable minus two"
meaning that the latest three releases of Rust will be supported. This
is enforced on CI with a full test suite job running on Linux x86_64
with the minimum supported Rust version. The full test suite will use
the latest stable version. A downside of this approach is that new
changes may break MSRV support on non-Linux or non-x86_64 platforms and
we won't know about it, but that's deemed a minor enough risk at this
time.

A minor fix is applied to Wasmtime's `Cargo.toml` to support Rust 1.70.0
instead of requiring Rust 1.71.0

* Fix installation of rust

* Scrape MSRV from Cargo.toml

* Cranelift is the same as Wasmtime's MSRV now, more words too

* Fix a typo

* aarch64: Use `RegScaled*` addressing modes (#6945)

This commit adds a few cases to `amode` construction on AArch64 for
using the `RegScaled*` variants of `AMode`. This won't affect wasm due
to this only matching the sign-extension happening before the shift, but
it should otherwise help non-wasm Cranelift use cases.

Closes #6742

* cranelift: Validate `iconst` ranges (#6850)

* cranelift: Validate `iconst` ranges

Add the following checks:

`iconst.i8`  immediate must be within 0 .. 2^8-1
`iconst.i16` immediate must be within 0 .. 2^16-1
`iconst.i32` immediate must be within 0 .. 2^32-1

Resolves #3059

* cranelift: Parse `iconst` according to its type

Modifies the parser for textual CLIF so that V in `iconst.T V` is
parsed according to T.

Before this commit, something like `iconst.i32 0xffff_ffff_ffff` was
valid because all `iconst` were parsed the same as an
`iconst.i64`. Now the above example will throw an error.

Also, a negative immediate as in `iconst.iN -X` is now converted to
`2^N - X`.

This commit also fixes some broken tests.

* cranelift: Update tests to match new CLIF parser

* Some minor fixes and features for WASI and sockets (#6948)

* Use `command::add_to_linker` in tests to reduce the number of times
  all the `add_to_linker` are listed.
* Add all `wasi:sockets` interfaces currently implemented to both the
  sync and async `command` functions (this enables all the interfaces in
  the CLI for example).
* Use `tokio::net::TcpStream::try_io` whenever I/O is performed on a
  socket, ensuring that readable/writable flags are set/cleared
  appropriately (otherwise once readable a socket is infinitely readable).
* Add a `with_ambient_tokio_runtime` helper function to use when
  creating a `tokio::net::TcpStream` since otherwise it panics due to a
  lack of active runtime in a synchronous context.
* Add `WouldBlock` handling to return a 0-length read.
* Add an `--inherit-network` CLI flag to enable basic usage of sockets
  in the CLI.

This will conflict a small amount with #6877 but should be easy to
resolve, and otherwise this targets different usability points/issues
than that PR.

---------

Co-authored-by: Afonso Bordado <afonso360@users.noreply.github.com>
Co-authored-by: Jamey Sharp <jsharp@fastly.com>
Co-authored-by: Timothée Jourde <timjrd@netc.fr>
Co-authored-by: Pat Hickey <phickey@fastly.com>
Co-authored-by: Trevor Elliott <telliott@fastly.com>
Co-authored-by: Dan Gohman <dev@sunfishcode.online>

Stop testing pseudocode

Restructure when notifications are sent, and make sure to flush the writer

Fix the wasi-http module versions of flush and blocking_flush

Use blocking_write_and_flush for blocking writes in the adapters

Fix a warning in wasi-http

Remove an unused DropPollable

add comment explaining try_write for tcpstream

refactor: separate struct for representing TcpReadStream

by factoring into HostTcpSocket a little bit

tcp read stream: handle stream closing

tcp tests: use blocking_read where its expecting to wait for input

move common test body into wasi-sockets-tests/src/lib.rs

ensure parent socket outlives pollable

input and output streams can be children now

tcp's streams are the sockets children

tcp.wit: document child relationships

tcp tests: fix to drop socket after its child streams

review feedback: propogate worker task panic

style

error source fix

tcp: use preview2::spawn, and propogate worker panics

join handle await always propogates panic

background task handles ewouldblock as well

document choice of constant
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 6, 2023
…6948)

* Use `command::add_to_linker` in tests to reduce the number of times
  all the `add_to_linker` are listed.
* Add all `wasi:sockets` interfaces currently implemented to both the
  sync and async `command` functions (this enables all the interfaces in
  the CLI for example).
* Use `tokio::net::TcpStream::try_io` whenever I/O is performed on a
  socket, ensuring that readable/writable flags are set/cleared
  appropriately (otherwise once readable a socket is infinitely readable).
* Add a `with_ambient_tokio_runtime` helper function to use when
  creating a `tokio::net::TcpStream` since otherwise it panics due to a
  lack of active runtime in a synchronous context.
* Add `WouldBlock` handling to return a 0-length read.
* Add an `--inherit-network` CLI flag to enable basic usage of sockets
  in the CLI.

This will conflict a small amount with bytecodealliance#6877 but should be easy to
resolve, and otherwise this targets different usability points/issues
than that PR.
* doc: document `HostOutputStream`

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Pat Hickey <pat@moreproductive.org>

* fix(wasi): fail when `MemoryOutputStream` buffer is full

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Pat Hickey <pat@moreproductive.org>
@pchickey pchickey marked this pull request as ready for review September 11, 2023 15:56
@pchickey pchickey requested review from a team as code owners September 11, 2023 15:56
@pchickey pchickey requested review from fitzgen and removed request for a team September 11, 2023 15:56
@elliottt elliottt added this pull request to the merge queue Sep 12, 2023
Merged via the queue into main with commit a0469b1 Sep 12, 2023
34 checks passed
@elliottt elliottt deleted the pch/backpressure_2 branch September 12, 2023 18:14
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Sep 12, 2023
With the merging of bytecodealliance#6877 prints to stdout with preview2 should now work
without requiring extra sleeps or such.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Sep 12, 2023
This was removed in bytecodealliance#6195 but re-added in bytecodealliance#6877, I believe by accident,
so this re-deletes it. I've also edited `.gitmodules` a bit while I was
here to remove it and additionally keep other entries up-to-date with
matching paths.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Sep 12, 2023
This was removed in bytecodealliance#6195 but re-added in bytecodealliance#6877, I believe by accident,
so this re-deletes it. I've also edited `.gitmodules` a bit while I was
here to remove it and additionally keep other entries up-to-date with
matching paths.
pchickey pushed a commit that referenced this pull request Sep 12, 2023
…6877)

* Stream backpressure v2

Co-authored-by: Pat Hickey <phickey@fastly.com>
Co-authored-by: Trevor Elliott <telliott@fastly.com>
Co-authored-by: Dan Gohman <dev@sunfishcode.online>

Stop testing pseudocode

Restructure when notifications are sent, and make sure to flush the writer

Fix the wasi-http module versions of flush and blocking_flush

Use blocking_write_and_flush for blocking writes in the adapters

Fix a warning in wasi-http

Remove an unused DropPollable

add comment explaining try_write for tcpstream

refactor: separate struct for representing TcpReadStream

by factoring into HostTcpSocket a little bit

tcp read stream: handle stream closing

tcp tests: use blocking_read where its expecting to wait for input

move common test body into wasi-sockets-tests/src/lib.rs

ensure parent socket outlives pollable

input and output streams can be children now

tcp's streams are the sockets children

tcp.wit: document child relationships

tcp tests: fix to drop socket after its child streams

review feedback: propogate worker task panic

style

error source fix

tcp: use preview2::spawn, and propogate worker panics

join handle await always propogates panic

background task handles ewouldblock as well

document choice of constant

* sync wit notes into wasi-http

* improve wit docs for output-stream

* doc: document `HostOutputStream` (#6980)

* doc: document `HostOutputStream`

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Pat Hickey <pat@moreproductive.org>

* fix(wasi): fail when `MemoryOutputStream` buffer is full

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Pat Hickey <pat@moreproductive.org>

* rustfmt

prtest:full

* windows and doc fixes

* cli test wasi-http: use blocking-write-and-flush

* Disable some tests, and adjust timeouts when running under qemu

* Try to reproduce the riscv64 failures

* Update riscv to LLVM 17 with beta rust

* Revert "Try to reproduce the riscv64 failures"

This reverts commit 8ac6781.

* Pin the beta version for riscv64

* Fix a warning on nightly

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Roman Volosatovs <rvolosatovs@users.noreply.github.com>
Co-authored-by: Trevor Elliott <telliott@fastly.com>
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
pchickey pushed a commit that referenced this pull request Sep 12, 2023
With the merging of #6877 prints to stdout with preview2 should now work
without requiring extra sleeps or such.
pchickey pushed a commit that referenced this pull request Sep 12, 2023
This was removed in #6195 but re-added in #6877, I believe by accident,
so this re-deletes it. I've also edited `.gitmodules` a bit while I was
here to remove it and additionally keep other entries up-to-date with
matching paths.
github-merge-queue bot pushed a commit that referenced this pull request Sep 12, 2023
With the merging of #6877 prints to stdout with preview2 should now work
without requiring extra sleeps or such.
github-merge-queue bot pushed a commit that referenced this pull request Sep 12, 2023
This was removed in #6195 but re-added in #6877, I believe by accident,
so this re-deletes it. I've also edited `.gitmodules` a bit while I was
here to remove it and additionally keep other entries up-to-date with
matching paths.
pchickey pushed a commit that referenced this pull request Sep 13, 2023
…13.0 (#7009)

* WASI preview 2 output-streams: new backpressure and flushing design (#6877)

* Stream backpressure v2

Co-authored-by: Pat Hickey <phickey@fastly.com>
Co-authored-by: Trevor Elliott <telliott@fastly.com>
Co-authored-by: Dan Gohman <dev@sunfishcode.online>

Stop testing pseudocode

Restructure when notifications are sent, and make sure to flush the writer

Fix the wasi-http module versions of flush and blocking_flush

Use blocking_write_and_flush for blocking writes in the adapters

Fix a warning in wasi-http

Remove an unused DropPollable

add comment explaining try_write for tcpstream

refactor: separate struct for representing TcpReadStream

by factoring into HostTcpSocket a little bit

tcp read stream: handle stream closing

tcp tests: use blocking_read where its expecting to wait for input

move common test body into wasi-sockets-tests/src/lib.rs

ensure parent socket outlives pollable

input and output streams can be children now

tcp's streams are the sockets children

tcp.wit: document child relationships

tcp tests: fix to drop socket after its child streams

review feedback: propogate worker task panic

style

error source fix

tcp: use preview2::spawn, and propogate worker panics

join handle await always propogates panic

background task handles ewouldblock as well

document choice of constant

* sync wit notes into wasi-http

* improve wit docs for output-stream

* doc: document `HostOutputStream` (#6980)

* doc: document `HostOutputStream`

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Pat Hickey <pat@moreproductive.org>

* fix(wasi): fail when `MemoryOutputStream` buffer is full

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Pat Hickey <pat@moreproductive.org>

* rustfmt

prtest:full

* windows and doc fixes

* cli test wasi-http: use blocking-write-and-flush

* Disable some tests, and adjust timeouts when running under qemu

* Try to reproduce the riscv64 failures

* Update riscv to LLVM 17 with beta rust

* Revert "Try to reproduce the riscv64 failures"

This reverts commit 8ac6781.

* Pin the beta version for riscv64

* Fix a warning on nightly

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Roman Volosatovs <rvolosatovs@users.noreply.github.com>
Co-authored-by: Trevor Elliott <telliott@fastly.com>
Co-authored-by: Alex Crichton <alex@alexcrichton.com>

* Un-ignore now-passing test

With the merging of #6877 prints to stdout with preview2 should now work
without requiring extra sleeps or such.

* Remove submodule re-added by accident

This was removed in #6195 but re-added in #6877, I believe by accident,
so this re-deletes it. I've also edited `.gitmodules` a bit while I was
here to remove it and additionally keep other entries up-to-date with
matching paths.

* add to release notes

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Roman Volosatovs <rvolosatovs@users.noreply.github.com>
Co-authored-by: Trevor Elliott <telliott@fastly.com>
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 13, 2023
…ytecodealliance#6877)

* Stream backpressure v2

Co-authored-by: Pat Hickey <phickey@fastly.com>
Co-authored-by: Trevor Elliott <telliott@fastly.com>
Co-authored-by: Dan Gohman <dev@sunfishcode.online>

Stop testing pseudocode

Restructure when notifications are sent, and make sure to flush the writer

Fix the wasi-http module versions of flush and blocking_flush

Use blocking_write_and_flush for blocking writes in the adapters

Fix a warning in wasi-http

Remove an unused DropPollable

add comment explaining try_write for tcpstream

refactor: separate struct for representing TcpReadStream

by factoring into HostTcpSocket a little bit

tcp read stream: handle stream closing

tcp tests: use blocking_read where its expecting to wait for input

move common test body into wasi-sockets-tests/src/lib.rs

ensure parent socket outlives pollable

input and output streams can be children now

tcp's streams are the sockets children

tcp.wit: document child relationships

tcp tests: fix to drop socket after its child streams

review feedback: propogate worker task panic

style

error source fix

tcp: use preview2::spawn, and propogate worker panics

join handle await always propogates panic

background task handles ewouldblock as well

document choice of constant

* sync wit notes into wasi-http

* improve wit docs for output-stream

* doc: document `HostOutputStream` (bytecodealliance#6980)

* doc: document `HostOutputStream`

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Pat Hickey <pat@moreproductive.org>

* fix(wasi): fail when `MemoryOutputStream` buffer is full

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Pat Hickey <pat@moreproductive.org>

* rustfmt

prtest:full

* windows and doc fixes

* cli test wasi-http: use blocking-write-and-flush

* Disable some tests, and adjust timeouts when running under qemu

* Try to reproduce the riscv64 failures

* Update riscv to LLVM 17 with beta rust

* Revert "Try to reproduce the riscv64 failures"

This reverts commit 8ac6781.

* Pin the beta version for riscv64

* Fix a warning on nightly

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Co-authored-by: Roman Volosatovs <rvolosatovs@users.noreply.github.com>
Co-authored-by: Trevor Elliott <telliott@fastly.com>
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 13, 2023
With the merging of bytecodealliance#6877 prints to stdout with preview2 should now work
without requiring extra sleeps or such.
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 13, 2023
This was removed in bytecodealliance#6195 but re-added in bytecodealliance#6877, I believe by accident,
so this re-deletes it. I've also edited `.gitmodules` a bit while I was
here to remove it and additionally keep other entries up-to-date with
matching paths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wasi Preview 2 Async Race Condition
4 participants