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-common support for tokio, & wiggle support for async methods containing sync code #2832

Merged
merged 73 commits into from
May 8, 2021

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Apr 13, 2021

This PR makes it possible for wasi-common to work on with async executor like tokio, without taking a dependency on any sort of asynchronous runtime. Existing users of wasi-common will not need to make any changes.

It introduces the new crate wasi-tokio, which is a sibling crate to wasi-cap-std-sync, and also depending on wasi-cap-std-sync, since much of the implementation has been reused.

wasmtime-wiggle now supports running async methods via a dummy executor, in order to support running synchronous wasi-common implementations. To configure this mode in wasmtime-wiggle, use the new block_on configuration option in place of async as introduced in #2701. This mode executes the async methods in a "dummy executor" - a mocked std::task::Waker which will only poll a future correctly if it immediately returns Poll::Ready.

wasi-common now uses an async_trait to make each WasiFile, WasiDir, and WasiSched trait method async. This means that any operation on files, directories, or the scheduler may return a pending future depending on the impl of those traits.

wasi-cap-std-sync stays the same, modulo the code golf required to add async in front of some fns. All of these impls are synchronous - the futures returned by these will always be Poll::Ready. Therefore, it is safe to use with the dummy executor.

The WasiFile trait has two additional methods, async fn readable(&mut self) -> Result<(), Error> and a corresponding writable, which have semantics corresponding to a wasi Subscription::FdRead and FdWrite - the futures are ready when the file is ready to read or write. These methods are intended to be used by an impl of WasiSched::poll_oneoff.

wasi-tokio is implemented mostly in terms of wasi-cap-std-sync. cap-std provides a robust implementation of filesystem sandboxing, which we still need to rely on. All file and directory operations delegated to the wasi-cap-std-sync impl are wrapped in tokio::task::block_in_place, which tells the tokio runtime to execute that code on a thread that may block. This is how tokio's own File type treats all of its IO operations as well. The departure of wasi-tokio from wasi-cap-std-sync is in the impl of the WasiFile::{readable, writable} methods, which use tokio::io::unix::AsyncFd when available, and the WasiSched::poll_oneoff method is implemented using those futures and tokio::time::timeout.

Unfortunately, tokio's AsyncFd doesn't do very much in this context on Linux (epoll(2) doesn't operate on regular files), and isn't available at all on Windows, but at least on Mac OS it does work well with kqueue(2). So, on Linux we are basically stuck making believe that regular files are immediately readable and writable, which is as much as select does for us anyway. The futures do work properly on pipes like stdin. On Windows, tokio doesn't give us anything like AsyncFd, so we fall back on using the cap-std-sync windows poll_oneoff inside a block_in_place.

wasmtime-wasi now exports two different Wasis - one under sync:: when the sync feature is enabled (which it is by default), and one under tokio:: when the tokio feature is enabled (not a default). The sync implementation is re-exported at the root of the crate, so that existing users do not have to change their code.

A new top-level examples/tokio shows using wasmtime_wasi::tokio::Wasi under tokio. It incorporates a lot of comments @alexcrichton wrote previously.

@pchickey pchickey changed the title wasi-common: support for running on async executors wasi-common: optional support for running on async executors Apr 13, 2021
@github-actions github-actions bot added the wasi Issues pertaining to WASI label Apr 13, 2021
@github-actions
Copy link

Subscribe to Label Action

cc @kubkon

This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

  • kubkon: wasi

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@github-actions github-actions bot added the wasmtime:c-api Issues pertaining to the C API. label Apr 14, 2021
@pchickey pchickey force-pushed the pch/wiggle_sync_shimming branch from 615a8f2 to 3d61c2a Compare April 14, 2021 00:51
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
@pchickey pchickey force-pushed the pch/wiggle_sync_shimming branch from 3d61c2a to 7590198 Compare April 14, 2021 00:53
@github-actions
Copy link

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:c-api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@pchickey pchickey changed the title wasi-common support for tokio, & wiggle support for async methods on sync code wasi-common support for tokio, & wiggle support for async methods containing sync code May 5, 2021
@pchickey pchickey requested a review from alexcrichton May 6, 2021 00:02
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.

👍 Nice!

I think there could be some more comments in a few places, and I've only somewhat lightly reviewed all the tokio scheduler bits but I think this looks pretty good. I keep finding it a bit backwards that all the sync stuff looks async but isn't, but it's all at least encapsulated.

crates/misc/run-examples/src/main.rs Outdated Show resolved Hide resolved
crates/test-programs/tests/wasm_tests/runtime/tokio.rs Outdated Show resolved Hide resolved
crates/test-programs/tests/wasm_tests/runtime/tokio.rs Outdated Show resolved Hide resolved
crates/wasi-common/cap-std-sync/Cargo.toml Outdated Show resolved Hide resolved
crates/wasi-common/tokio/src/lib.rs Show resolved Hide resolved
crates/wasi-common/tokio/src/sched/windows.rs Outdated Show resolved Hide resolved
crates/wiggle/src/lib.rs Outdated Show resolved Hide resolved
examples/tokio/main.rs Show resolved Hide resolved
examples/tokio/main.rs Outdated Show resolved Hide resolved
@pchickey pchickey requested a review from alexcrichton May 6, 2021 23:25
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.

Ok all looks good to me! I think it's ok to leave the windows poll issue to, well, an issue for now. Other than that seems good to go for me! (although there's some CI failures)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI wasmtime:c-api Issues pertaining to the C API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants