From fcf5f94aab80303f107381a241737ed26afbf0ab Mon Sep 17 00:00:00 2001 From: Harald Hoyer Date: Fri, 25 Nov 2022 15:35:54 +0100 Subject: [PATCH 01/33] feat: remove mutability from the WasiCtx Table This patch adds interior mutability to the WasiCtx Table and the Table elements. Major pain points: * `File` only needs `RwLock` to implement `File::set_fdflags()` on Windows, because of [1] * Because `File` needs a `RwLock` and `RwLock*Guard` cannot be hold across an `.await`, The `async` from `async fn num_ready_bytes(&self)` had to be removed * Because `File` needs a `RwLock` and `RwLock*Guard` cannot be dereferenced in `pollable`, the signature of `fn pollable(&self) -> Option` changed to `fn pollable(&self) -> Option>` [1] https://github.com/bytecodealliance/system-interface/blob/da238e324e752033f315f09c082ad9ce35d42696/src/fs/fd_flags.rs#L210-L217 Signed-off-by: Harald Hoyer --- crates/wasi-common/cap-std-sync/src/file.rs | 160 +++++++++------- crates/wasi-common/cap-std-sync/src/lib.rs | 10 +- crates/wasi-common/cap-std-sync/src/net.rs | 56 +++--- .../cap-std-sync/src/sched/unix.rs | 13 +- .../cap-std-sync/src/sched/windows.rs | 2 +- crates/wasi-common/cap-std-sync/src/stdio.rs | 55 +++--- crates/wasi-common/src/ctx.rs | 33 ++-- crates/wasi-common/src/dir.rs | 93 +++++---- crates/wasi-common/src/file.rs | 128 ++++++++----- crates/wasi-common/src/pipe.rs | 10 +- crates/wasi-common/src/snapshots/preview_0.rs | 34 ++-- crates/wasi-common/src/snapshots/preview_1.rs | 177 +++++++++--------- crates/wasi-common/src/table.rs | 70 +++---- crates/wasi-common/tokio/src/file.rs | 57 +++--- crates/wasi-common/tokio/src/lib.rs | 10 +- crates/wasi-common/tokio/src/sched/unix.rs | 1 - crates/wasi-common/tokio/tests/poll_oneoff.rs | 2 +- 17 files changed, 491 insertions(+), 420 deletions(-) diff --git a/crates/wasi-common/cap-std-sync/src/file.rs b/crates/wasi-common/cap-std-sync/src/file.rs index 8fe395fdaf8b..c54a5ead7145 100644 --- a/crates/wasi-common/cap-std-sync/src/file.rs +++ b/crates/wasi-common/cap-std-sync/src/file.rs @@ -5,6 +5,7 @@ use is_terminal::IsTerminal; use std::any::Any; use std::convert::TryInto; use std::io; +use std::sync::{Arc, RwLock, RwLockReadGuard}; use system_interface::{ fs::{FileIoExt, GetSetFdFlags}, io::{IoExt, ReadReady}, @@ -14,11 +15,48 @@ use wasi_common::{ Error, ErrorExt, }; -pub struct File(cap_std::fs::File); +#[cfg(unix)] +use io_lifetimes::{AsFd, BorrowedFd}; + +#[cfg(windows)] +use io_lifetimes::{AsHandle, BorrowedHandle}; + +#[cfg(windows)] +use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; + +pub struct BorrowedFile<'a>(RwLockReadGuard<'a, cap_std::fs::File>); + +#[cfg(unix)] +impl AsFd for BorrowedFile<'_> { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0.as_fd() + } +} + +#[cfg(windows)] +impl AsHandle for BorrowedFile<'_> { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.0.as_handle() + } +} + +#[cfg(windows)] +impl AsRawHandleOrSocket for BorrowedFile<'_> { + #[inline] + fn as_raw_handle_or_socket(&self) -> RawHandleOrSocket { + self.0.as_raw_handle_or_socket() + } +} + +pub struct File(RwLock); impl File { pub fn from_cap_std(file: cap_std::fs::File) -> Self { - File(file) + File(RwLock::new(file)) + } + + pub fn borrow(&self) -> BorrowedFile { + BorrowedFile(self.0.read().unwrap()) } } @@ -27,32 +65,35 @@ impl WasiFile for File { fn as_any(&self) -> &dyn Any { self } + #[cfg(unix)] - fn pollable(&self) -> Option { - Some(self.0.as_fd()) + fn pollable(&self) -> Option> { + Some(Arc::new(self.borrow())) } #[cfg(windows)] - fn pollable(&self) -> Option { - Some(self.0.as_raw_handle_or_socket()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedFile(self.0.read().unwrap()))) } - async fn datasync(&mut self) -> Result<(), Error> { - self.0.sync_data()?; + + async fn datasync(&self) -> Result<(), Error> { + self.0.read().unwrap().sync_data()?; Ok(()) } - async fn sync(&mut self) -> Result<(), Error> { - self.0.sync_all()?; + async fn sync(&self) -> Result<(), Error> { + self.0.read().unwrap().sync_all()?; Ok(()) } - async fn get_filetype(&mut self) -> Result { - let meta = self.0.metadata()?; + async fn get_filetype(&self) -> Result { + let meta = self.0.read().unwrap().metadata()?; Ok(filetype_from(&meta.file_type())) } - async fn get_fdflags(&mut self) -> Result { - let fdflags = get_fd_flags(&self.0)?; + async fn get_fdflags(&self) -> Result { + let file = self.0.read().unwrap(); + let fdflags = get_fd_flags(&*file)?; Ok(fdflags) } - async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&self, fdflags: FdFlags) -> Result<(), Error> { if fdflags.intersects( wasi_common::file::FdFlags::DSYNC | wasi_common::file::FdFlags::SYNC @@ -60,12 +101,13 @@ impl WasiFile for File { ) { return Err(Error::invalid_argument().context("cannot set DSYNC, SYNC, or RSYNC flag")); } - let set_fd_flags = self.0.new_set_fd_flags(to_sysif_fdflags(fdflags))?; - self.0.set_fd_flags(set_fd_flags)?; + let mut file = self.0.write().unwrap(); + let set_fd_flags = (*file).new_set_fd_flags(to_sysif_fdflags(fdflags))?; + (*file).set_fd_flags(set_fd_flags)?; Ok(()) } - async fn get_filestat(&mut self) -> Result { - let meta = self.0.metadata()?; + async fn get_filestat(&self) -> Result { + let meta = self.0.read().unwrap().metadata()?; Ok(Filestat { device_id: meta.dev(), inode: meta.ino(), @@ -77,63 +119,68 @@ impl WasiFile for File { ctim: meta.created().map(|t| Some(t.into_std())).unwrap_or(None), }) } - async fn set_filestat_size(&mut self, size: u64) -> Result<(), Error> { - self.0.set_len(size)?; + async fn set_filestat_size(&self, size: u64) -> Result<(), Error> { + self.0.read().unwrap().set_len(size)?; Ok(()) } - async fn advise(&mut self, offset: u64, len: u64, advice: Advice) -> Result<(), Error> { - self.0.advise(offset, len, convert_advice(advice))?; + async fn advise(&self, offset: u64, len: u64, advice: Advice) -> Result<(), Error> { + self.0 + .read() + .unwrap() + .advise(offset, len, convert_advice(advice))?; Ok(()) } - async fn allocate(&mut self, offset: u64, len: u64) -> Result<(), Error> { - self.0.allocate(offset, len)?; + async fn allocate(&self, offset: u64, len: u64) -> Result<(), Error> { + self.0.read().unwrap().allocate(offset, len)?; Ok(()) } async fn set_times( - &mut self, + &self, atime: Option, mtime: Option, ) -> Result<(), Error> { self.0 + .read() + .unwrap() .set_times(convert_systimespec(atime), convert_systimespec(mtime))?; Ok(()) } - async fn read_vectored<'a>(&mut self, bufs: &mut [io::IoSliceMut<'a>]) -> Result { - let n = self.0.read_vectored(bufs)?; + async fn read_vectored<'a>(&self, bufs: &mut [io::IoSliceMut<'a>]) -> Result { + let n = self.0.read().unwrap().read_vectored(bufs)?; Ok(n.try_into()?) } async fn read_vectored_at<'a>( - &mut self, + &self, bufs: &mut [io::IoSliceMut<'a>], offset: u64, ) -> Result { - let n = self.0.read_vectored_at(bufs, offset)?; + let n = self.0.read().unwrap().read_vectored_at(bufs, offset)?; Ok(n.try_into()?) } - async fn write_vectored<'a>(&mut self, bufs: &[io::IoSlice<'a>]) -> Result { - let n = self.0.write_vectored(bufs)?; + async fn write_vectored<'a>(&self, bufs: &[io::IoSlice<'a>]) -> Result { + let n = self.0.read().unwrap().write_vectored(bufs)?; Ok(n.try_into()?) } async fn write_vectored_at<'a>( - &mut self, + &self, bufs: &[io::IoSlice<'a>], offset: u64, ) -> Result { - let n = self.0.write_vectored_at(bufs, offset)?; + let n = self.0.read().unwrap().write_vectored_at(bufs, offset)?; Ok(n.try_into()?) } - async fn seek(&mut self, pos: std::io::SeekFrom) -> Result { - Ok(self.0.seek(pos)?) + async fn seek(&self, pos: std::io::SeekFrom) -> Result { + Ok(self.0.read().unwrap().seek(pos)?) } - async fn peek(&mut self, buf: &mut [u8]) -> Result { - let n = self.0.peek(buf)?; + async fn peek(&self, buf: &mut [u8]) -> Result { + let n = self.0.read().unwrap().peek(buf)?; Ok(n.try_into()?) } - async fn num_ready_bytes(&self) -> Result { - Ok(self.0.num_ready_bytes()?) + fn num_ready_bytes(&self) -> Result { + Ok(self.0.read().unwrap().num_ready_bytes()?) } - fn isatty(&mut self) -> bool { - self.0.is_terminal() + fn isatty(&self) -> bool { + self.0.read().unwrap().is_terminal() } } @@ -160,35 +207,6 @@ pub fn filetype_from(ft: &cap_std::fs::FileType) -> FileType { } } -#[cfg(windows)] -use io_lifetimes::{AsHandle, BorrowedHandle}; -#[cfg(windows)] -impl AsHandle for File { - fn as_handle(&self) -> BorrowedHandle<'_> { - self.0.as_handle() - } -} - -#[cfg(windows)] -use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; -#[cfg(windows)] -impl AsRawHandleOrSocket for File { - #[inline] - fn as_raw_handle_or_socket(&self) -> RawHandleOrSocket { - self.0.as_raw_handle_or_socket() - } -} - -#[cfg(unix)] -use io_lifetimes::{AsFd, BorrowedFd}; - -#[cfg(unix)] -impl AsFd for File { - fn as_fd(&self) -> BorrowedFd<'_> { - self.0.as_fd() - } -} - pub(crate) fn convert_systimespec( t: Option, ) -> Option { diff --git a/crates/wasi-common/cap-std-sync/src/lib.rs b/crates/wasi-common/cap-std-sync/src/lib.rs index 12f27bcec885..fbaa7bbbd9d9 100644 --- a/crates/wasi-common/cap-std-sync/src/lib.rs +++ b/crates/wasi-common/cap-std-sync/src/lib.rs @@ -94,15 +94,15 @@ impl WasiCtxBuilder { } Ok(self) } - pub fn stdin(mut self, f: Box) -> Self { + pub fn stdin(self, f: Box) -> Self { self.0.set_stdin(f); self } - pub fn stdout(mut self, f: Box) -> Self { + pub fn stdout(self, f: Box) -> Self { self.0.set_stdout(f); self } - pub fn stderr(mut self, f: Box) -> Self { + pub fn stderr(self, f: Box) -> Self { self.0.set_stderr(f); self } @@ -118,12 +118,12 @@ impl WasiCtxBuilder { pub fn inherit_stdio(self) -> Self { self.inherit_stdin().inherit_stdout().inherit_stderr() } - pub fn preopened_dir(mut self, dir: Dir, guest_path: impl AsRef) -> Result { + pub fn preopened_dir(self, dir: Dir, guest_path: impl AsRef) -> Result { let dir = Box::new(crate::dir::Dir::from_cap_std(dir)); self.0.push_preopened_dir(dir, guest_path)?; Ok(self) } - pub fn preopened_socket(mut self, fd: u32, socket: impl Into) -> Result { + pub fn preopened_socket(self, fd: u32, socket: impl Into) -> Result { let socket: Socket = socket.into(); let file: Box = socket.into(); diff --git a/crates/wasi-common/cap-std-sync/src/net.rs b/crates/wasi-common/cap-std-sync/src/net.rs index bdbf507fe48a..b46d8d7dafaf 100644 --- a/crates/wasi-common/cap-std-sync/src/net.rs +++ b/crates/wasi-common/cap-std-sync/src/net.rs @@ -8,6 +8,7 @@ use io_lifetimes::{AsSocket, BorrowedSocket}; use std::any::Any; use std::convert::TryInto; use std::io; +use std::sync::Arc; #[cfg(unix)] use system_interface::fs::GetSetFdFlags; use system_interface::io::IoExt; @@ -18,6 +19,11 @@ use wasi_common::{ Error, ErrorExt, }; +#[cfg(unix)] +use wasi_common::file::BorrowedAsFd; +#[cfg(windows)] +use wasi_common::file::BorrowedAsRawHandleOrSocket; + pub enum Socket { TcpListener(cap_std::net::TcpListener), TcpStream(cap_std::net::TcpStream), @@ -83,29 +89,28 @@ macro_rules! wasi_listen_write_impl { self } #[cfg(unix)] - fn pollable(&self) -> Option { - Some(self.0.as_fd()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedAsFd::new(&self.0))) } - #[cfg(windows)] - fn pollable(&self) -> Option { - Some(self.0.as_raw_handle_or_socket()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedAsRawHandleOrSocket::new(&self.0))) } - async fn sock_accept(&mut self, fdflags: FdFlags) -> Result, Error> { + async fn sock_accept(&self, fdflags: FdFlags) -> Result, Error> { let (stream, _) = self.0.accept()?; - let mut stream = <$stream>::from_cap_std(stream); + let stream = <$stream>::from_cap_std(stream); stream.set_fdflags(fdflags).await?; Ok(Box::new(stream)) } - async fn get_filetype(&mut self) -> Result { + async fn get_filetype(&self) -> Result { Ok(FileType::SocketStream) } #[cfg(unix)] - async fn get_fdflags(&mut self) -> Result { + async fn get_fdflags(&self) -> Result { let fdflags = get_fd_flags(&self.0)?; Ok(fdflags) } - async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&self, fdflags: FdFlags) -> Result<(), Error> { if fdflags == wasi_common::file::FdFlags::NONBLOCK { self.0.set_nonblocking(true)?; } else if fdflags.is_empty() { @@ -117,7 +122,7 @@ macro_rules! wasi_listen_write_impl { } Ok(()) } - async fn num_ready_bytes(&self) -> Result { + fn num_ready_bytes(&self) -> Result { Ok(1) } } @@ -177,23 +182,22 @@ macro_rules! wasi_stream_write_impl { self } #[cfg(unix)] - fn pollable(&self) -> Option { - Some(self.0.as_fd()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedAsFd::new(&self.0))) } - #[cfg(windows)] - fn pollable(&self) -> Option { - Some(self.0.as_raw_handle_or_socket()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedAsRawHandleOrSocket::new(&self.0))) } - async fn get_filetype(&mut self) -> Result { + async fn get_filetype(&self) -> Result { Ok(FileType::SocketStream) } #[cfg(unix)] - async fn get_fdflags(&mut self) -> Result { + async fn get_fdflags(&self) -> Result { let fdflags = get_fd_flags(&self.0)?; Ok(fdflags) } - async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&self, fdflags: FdFlags) -> Result<(), Error> { if fdflags == wasi_common::file::FdFlags::NONBLOCK { self.0.set_nonblocking(true)?; } else if fdflags.is_empty() { @@ -206,23 +210,23 @@ macro_rules! wasi_stream_write_impl { Ok(()) } async fn read_vectored<'a>( - &mut self, + &self, bufs: &mut [io::IoSliceMut<'a>], ) -> Result { use std::io::Read; let n = Read::read_vectored(&mut &*self.as_socketlike_view::<$std_ty>(), bufs)?; Ok(n.try_into()?) } - async fn write_vectored<'a>(&mut self, bufs: &[io::IoSlice<'a>]) -> Result { + async fn write_vectored<'a>(&self, bufs: &[io::IoSlice<'a>]) -> Result { use std::io::Write; let n = Write::write_vectored(&mut &*self.as_socketlike_view::<$std_ty>(), bufs)?; Ok(n.try_into()?) } - async fn peek(&mut self, buf: &mut [u8]) -> Result { + async fn peek(&self, buf: &mut [u8]) -> Result { let n = self.0.peek(buf)?; Ok(n.try_into()?) } - async fn num_ready_bytes(&self) -> Result { + fn num_ready_bytes(&self) -> Result { let val = self.as_socketlike_view::<$std_ty>().num_ready_bytes()?; Ok(val) } @@ -244,7 +248,7 @@ macro_rules! wasi_stream_write_impl { } async fn sock_recv<'a>( - &mut self, + &self, ri_data: &mut [std::io::IoSliceMut<'a>], ri_flags: RiFlags, ) -> Result<(u64, RoFlags), Error> { @@ -272,7 +276,7 @@ macro_rules! wasi_stream_write_impl { } async fn sock_send<'a>( - &mut self, + &self, si_data: &[std::io::IoSlice<'a>], si_flags: SiFlags, ) -> Result { @@ -284,7 +288,7 @@ macro_rules! wasi_stream_write_impl { Ok(n as u64) } - async fn sock_shutdown(&mut self, how: SdFlags) -> Result<(), Error> { + async fn sock_shutdown(&self, how: SdFlags) -> Result<(), Error> { let how = if how == SdFlags::RD | SdFlags::WR { cap_std::net::Shutdown::Both } else if how == SdFlags::RD { diff --git a/crates/wasi-common/cap-std-sync/src/sched/unix.rs b/crates/wasi-common/cap-std-sync/src/sched/unix.rs index c53acf1a29f8..000299e2721b 100644 --- a/crates/wasi-common/cap-std-sync/src/sched/unix.rs +++ b/crates/wasi-common/cap-std-sync/src/sched/unix.rs @@ -8,7 +8,7 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> { if poll.is_empty() { return Ok(()); } - let mut pollfds = Vec::new(); + let mut poll_as_fds = Vec::new(); for s in poll.rw_subscriptions() { match s { Subscription::Read(f) => { @@ -16,7 +16,7 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> { .file .pollable() .ok_or(Error::invalid_argument().context("file is not pollable"))?; - pollfds.push(PollFd::from_borrowed_fd(fd, PollFlags::IN)); + poll_as_fds.push((fd, PollFlags::IN)); } Subscription::Write(f) => { @@ -24,12 +24,17 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> { .file .pollable() .ok_or(Error::invalid_argument().context("file is not pollable"))?; - pollfds.push(PollFd::from_borrowed_fd(fd, PollFlags::OUT)); + poll_as_fds.push((fd, PollFlags::OUT)); } Subscription::MonotonicClock { .. } => unreachable!(), } } + let mut pollfds = poll_as_fds + .iter() + .map(|(fd, events)| PollFd::from_borrowed_fd(fd.as_fd(), events.clone())) + .collect::>(); + let ready = loop { let poll_timeout = if let Some(t) = poll.earliest_clock_deadline() { let duration = t.duration_until().unwrap_or(Duration::from_secs(0)); @@ -55,7 +60,7 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> { let revents = pollfd.revents(); let (nbytes, rwsub) = match rwsub { Subscription::Read(sub) => { - let ready = sub.file.num_ready_bytes().await?; + let ready = sub.file.num_ready_bytes()?; (std::cmp::max(ready, 1), sub) } Subscription::Write(sub) => (0, sub), diff --git a/crates/wasi-common/cap-std-sync/src/sched/windows.rs b/crates/wasi-common/cap-std-sync/src/sched/windows.rs index c4ad559cc344..e3eeb930523e 100644 --- a/crates/wasi-common/cap-std-sync/src/sched/windows.rs +++ b/crates/wasi-common/cap-std-sync/src/sched/windows.rs @@ -96,7 +96,7 @@ pub async fn poll_oneoff_<'a>( } } for r in immediate_reads { - match r.file.num_ready_bytes().await { + match r.file.num_ready_bytes() { Ok(ready_bytes) => { r.complete(ready_bytes, RwEventFlags::empty()); ready = true; diff --git a/crates/wasi-common/cap-std-sync/src/stdio.rs b/crates/wasi-common/cap-std-sync/src/stdio.rs index 9d82348af15d..dd6f2344faf4 100644 --- a/crates/wasi-common/cap-std-sync/src/stdio.rs +++ b/crates/wasi-common/cap-std-sync/src/stdio.rs @@ -7,8 +7,14 @@ use std::convert::TryInto; use std::fs::File; use std::io; use std::io::{Read, Write}; +use std::sync::Arc; use system_interface::io::ReadReady; +#[cfg(unix)] +use wasi_common::file::BorrowedAsFd; +#[cfg(windows)] +use wasi_common::file::BorrowedAsRawHandleOrSocket; + #[cfg(windows)] use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; #[cfg(unix)] @@ -31,41 +37,43 @@ impl WasiFile for Stdin { fn as_any(&self) -> &dyn Any { self } + #[cfg(unix)] - fn pollable(&self) -> Option { - Some(self.0.as_fd()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedAsFd::new(&self.0))) } #[cfg(windows)] - fn pollable(&self) -> Option { - Some(self.0.as_raw_handle_or_socket()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedAsRawHandleOrSocket::new(&self.0))) } - async fn get_filetype(&mut self) -> Result { + + async fn get_filetype(&self) -> Result { if self.isatty() { Ok(FileType::CharacterDevice) } else { Ok(FileType::Unknown) } } - async fn read_vectored<'a>(&mut self, bufs: &mut [io::IoSliceMut<'a>]) -> Result { + async fn read_vectored<'a>(&self, bufs: &mut [io::IoSliceMut<'a>]) -> Result { let n = (&*self.0.as_filelike_view::()).read_vectored(bufs)?; Ok(n.try_into().map_err(|_| Error::range())?) } async fn read_vectored_at<'a>( - &mut self, + &self, _bufs: &mut [io::IoSliceMut<'a>], _offset: u64, ) -> Result { Err(Error::seek_pipe()) } - async fn seek(&mut self, _pos: std::io::SeekFrom) -> Result { + async fn seek(&self, _pos: std::io::SeekFrom) -> Result { Err(Error::seek_pipe()) } - async fn peek(&mut self, _buf: &mut [u8]) -> Result { + async fn peek(&self, _buf: &mut [u8]) -> Result { Err(Error::seek_pipe()) } async fn set_times( - &mut self, + &self, atime: Option, mtime: Option, ) -> Result<(), Error> { @@ -73,10 +81,10 @@ impl WasiFile for Stdin { .set_times(convert_systimespec(atime), convert_systimespec(mtime))?; Ok(()) } - async fn num_ready_bytes(&self) -> Result { + fn num_ready_bytes(&self) -> Result { Ok(self.0.num_ready_bytes()?) } - fn isatty(&mut self) -> bool { + fn isatty(&self) -> bool { self.0.is_terminal() } } @@ -108,42 +116,41 @@ macro_rules! wasi_file_write_impl { self } #[cfg(unix)] - fn pollable(&self) -> Option { - Some(self.0.as_fd()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedAsFd::new(&self.0))) } - #[cfg(windows)] - fn pollable(&self) -> Option { - Some(self.0.as_raw_handle_or_socket()) + fn pollable(&self) -> Option> { + Some(Arc::new(BorrowedAsRawHandleOrSocket::new(&self.0))) } - async fn get_filetype(&mut self) -> Result { + async fn get_filetype(&self) -> Result { if self.isatty() { Ok(FileType::CharacterDevice) } else { Ok(FileType::Unknown) } } - async fn get_fdflags(&mut self) -> Result { + async fn get_fdflags(&self) -> Result { Ok(FdFlags::APPEND) } - async fn write_vectored<'a>(&mut self, bufs: &[io::IoSlice<'a>]) -> Result { + async fn write_vectored<'a>(&self, bufs: &[io::IoSlice<'a>]) -> Result { let n = (&*self.0.as_filelike_view::()).write_vectored(bufs)?; Ok(n.try_into().map_err(|_| { Error::range().context("converting write_vectored total length") })?) } async fn write_vectored_at<'a>( - &mut self, + &self, _bufs: &[io::IoSlice<'a>], _offset: u64, ) -> Result { Err(Error::seek_pipe()) } - async fn seek(&mut self, _pos: std::io::SeekFrom) -> Result { + async fn seek(&self, _pos: std::io::SeekFrom) -> Result { Err(Error::seek_pipe()) } async fn set_times( - &mut self, + &self, atime: Option, mtime: Option, ) -> Result<(), Error> { @@ -151,7 +158,7 @@ macro_rules! wasi_file_write_impl { .set_times(convert_systimespec(atime), convert_systimespec(mtime))?; Ok(()) } - fn isatty(&mut self) -> bool { + fn isatty(&self) -> bool { self.0.is_terminal() } } diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index b4b3a55324a7..1a115e0d8d43 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -7,6 +7,7 @@ use crate::table::Table; use crate::Error; use cap_rand::RngCore; use std::path::{Path, PathBuf}; +use std::sync::Arc; pub struct WasiCtx { pub args: StringArray, @@ -24,7 +25,7 @@ impl WasiCtx { sched: Box, table: Table, ) -> Self { - let mut s = WasiCtx { + let s = WasiCtx { args: StringArray::new(), env: StringArray::new(), random, @@ -38,17 +39,17 @@ impl WasiCtx { s } - pub fn insert_file(&mut self, fd: u32, file: Box, caps: FileCaps) { + pub fn insert_file(&self, fd: u32, file: Box, caps: FileCaps) { self.table() - .insert_at(fd, Box::new(FileEntry::new(caps, file))); + .insert_at(fd, Arc::new(FileEntry::new(caps, file))); } - pub fn push_file(&mut self, file: Box, caps: FileCaps) -> Result { - self.table().push(Box::new(FileEntry::new(caps, file))) + pub fn push_file(&self, file: Box, caps: FileCaps) -> Result { + self.table().push(Arc::new(FileEntry::new(caps, file))) } pub fn insert_dir( - &mut self, + &self, fd: u32, dir: Box, caps: DirCaps, @@ -57,23 +58,23 @@ impl WasiCtx { ) { self.table().insert_at( fd, - Box::new(DirEntry::new(caps, file_caps, Some(path), dir)), + Arc::new(DirEntry::new(caps, file_caps, Some(path), dir)), ); } pub fn push_dir( - &mut self, + &self, dir: Box, caps: DirCaps, file_caps: FileCaps, path: PathBuf, ) -> Result { self.table() - .push(Box::new(DirEntry::new(caps, file_caps, Some(path), dir))) + .push(Arc::new(DirEntry::new(caps, file_caps, Some(path), dir))) } - pub fn table(&mut self) -> &mut Table { - &mut self.table + pub fn table(&self) -> &Table { + &self.table } pub fn push_arg(&mut self, arg: &str) -> Result<(), StringArrayError> { @@ -85,17 +86,17 @@ impl WasiCtx { Ok(()) } - pub fn set_stdin(&mut self, mut f: Box) { + pub fn set_stdin(&self, mut f: Box) { let rights = Self::stdio_rights(&mut *f); self.insert_file(0, f, rights); } - pub fn set_stdout(&mut self, mut f: Box) { + pub fn set_stdout(&self, mut f: Box) { let rights = Self::stdio_rights(&mut *f); self.insert_file(1, f, rights); } - pub fn set_stderr(&mut self, mut f: Box) { + pub fn set_stderr(&self, mut f: Box) { let rights = Self::stdio_rights(&mut *f); self.insert_file(2, f, rights); } @@ -114,13 +115,13 @@ impl WasiCtx { } pub fn push_preopened_dir( - &mut self, + &self, dir: Box, path: impl AsRef, ) -> Result<(), Error> { let caps = DirCaps::all(); let file_caps = FileCaps::all(); - self.table().push(Box::new(DirEntry::new( + self.table().push(Arc::new(DirEntry::new( caps, file_caps, Some(path.as_ref().to_owned()), diff --git a/crates/wasi-common/src/dir.rs b/crates/wasi-common/src/dir.rs index 56a8849db468..48cc10d2f604 100644 --- a/crates/wasi-common/src/dir.rs +++ b/crates/wasi-common/src/dir.rs @@ -3,6 +3,7 @@ use crate::{Error, ErrorExt, SystemTimeSpec}; use bitflags::bitflags; use std::any::Any; use std::path::PathBuf; +use std::sync::{Arc, RwLock}; #[wiggle::async_trait] pub trait WasiDir: Send + Sync { @@ -98,67 +99,50 @@ pub trait WasiDir: Send + Sync { } pub(crate) struct DirEntry { - caps: DirCaps, - file_caps: FileCaps, + caps: RwLock, preopen_path: Option, // precondition: PathBuf is valid unicode dir: Box, } impl DirEntry { pub fn new( - caps: DirCaps, + dir_caps: DirCaps, file_caps: FileCaps, preopen_path: Option, dir: Box, ) -> Self { DirEntry { - caps, - file_caps, + caps: RwLock::new(DirFdStat { + dir_caps, + file_caps, + }), preopen_path, dir, } } pub fn capable_of_dir(&self, caps: DirCaps) -> Result<(), Error> { - if self.caps.contains(caps) { - Ok(()) - } else { - let missing = caps & !self.caps; - let err = if missing.intersects(DirCaps::READDIR) { - Error::not_dir() - } else { - Error::perm() - }; - Err(err.context(format!("desired rights {:?}, has {:?}", caps, self.caps))) - } - } - pub fn capable_of_file(&self, caps: FileCaps) -> Result<(), Error> { - if self.file_caps.contains(caps) { - Ok(()) - } else { - Err(Error::perm().context(format!( - "desired rights {:?}, has {:?}", - caps, self.file_caps - ))) - } + let fdstat = self.caps.read().unwrap(); + fdstat.capable_of_dir(caps) } - pub fn drop_caps_to(&mut self, caps: DirCaps, file_caps: FileCaps) -> Result<(), Error> { - self.capable_of_dir(caps)?; - self.capable_of_file(file_caps)?; - self.caps = caps; - self.file_caps = file_caps; + + pub fn drop_caps_to(&self, dir_caps: DirCaps, file_caps: FileCaps) -> Result<(), Error> { + let mut fdstat = self.caps.write().unwrap(); + fdstat.capable_of_dir(dir_caps)?; + fdstat.capable_of_file(file_caps)?; + *fdstat = DirFdStat { + dir_caps, + file_caps, + }; Ok(()) } pub fn child_dir_caps(&self, desired_caps: DirCaps) -> DirCaps { - self.caps & desired_caps + self.caps.read().unwrap().dir_caps & desired_caps } pub fn child_file_caps(&self, desired_caps: FileCaps) -> FileCaps { - self.file_caps & desired_caps + self.caps.read().unwrap().file_caps & desired_caps } pub fn get_dir_fdstat(&self) -> DirFdStat { - DirFdStat { - dir_caps: self.caps, - file_caps: self.file_caps, - } + self.caps.read().unwrap().clone() } pub fn preopen_path(&self) -> &Option { &self.preopen_path @@ -203,18 +187,47 @@ pub struct DirFdStat { pub dir_caps: DirCaps, } +impl DirFdStat { + pub fn capable_of_dir(&self, caps: DirCaps) -> Result<(), Error> { + if self.dir_caps.contains(caps) { + Ok(()) + } else { + let missing = caps & !self.dir_caps; + let err = if missing.intersects(DirCaps::READDIR) { + Error::not_dir() + } else { + Error::perm() + }; + Err(err.context(format!( + "desired rights {:?}, has {:?}", + caps, self.dir_caps + ))) + } + } + pub fn capable_of_file(&self, caps: FileCaps) -> Result<(), Error> { + if self.file_caps.contains(caps) { + Ok(()) + } else { + Err(Error::perm().context(format!( + "desired rights {:?}, has {:?}", + caps, self.file_caps + ))) + } + } +} + pub(crate) trait TableDirExt { - fn get_dir(&self, fd: u32) -> Result<&DirEntry, Error>; + fn get_dir(&self, fd: u32) -> Result, Error>; fn is_preopen(&self, fd: u32) -> bool; } impl TableDirExt for crate::table::Table { - fn get_dir(&self, fd: u32) -> Result<&DirEntry, Error> { + fn get_dir(&self, fd: u32) -> Result, Error> { self.get(fd) } fn is_preopen(&self, fd: u32) -> bool { if self.is::(fd) { - let dir_entry: &DirEntry = self.get(fd).unwrap(); + let dir_entry: Arc = self.get(fd).unwrap(); dir_entry.preopen_path.is_some() } else { false diff --git a/crates/wasi-common/src/file.rs b/crates/wasi-common/src/file.rs index 8d4bf6a45a61..b76278373a65 100644 --- a/crates/wasi-common/src/file.rs +++ b/crates/wasi-common/src/file.rs @@ -1,32 +1,39 @@ use crate::{Error, ErrorExt, SystemTimeSpec}; use bitflags::bitflags; use std::any::Any; +use std::sync::{Arc, RwLock}; + +#[cfg(unix)] +use cap_std::io_lifetimes::{AsFd, BorrowedFd}; + +#[cfg(windows)] +use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; #[wiggle::async_trait] pub trait WasiFile: Send + Sync { fn as_any(&self) -> &dyn Any; - async fn get_filetype(&mut self) -> Result; + async fn get_filetype(&self) -> Result; #[cfg(unix)] - fn pollable(&self) -> Option { + fn pollable(&self) -> Option> { None } #[cfg(windows)] - fn pollable(&self) -> Option { + fn pollable(&self) -> Option> { None } - fn isatty(&mut self) -> bool { + fn isatty(&self) -> bool { false } - async fn sock_accept(&mut self, _fdflags: FdFlags) -> Result, Error> { + async fn sock_accept(&self, _fdflags: FdFlags) -> Result, Error> { Err(Error::badf()) } async fn sock_recv<'a>( - &mut self, + &self, _ri_data: &mut [std::io::IoSliceMut<'a>], _ri_flags: RiFlags, ) -> Result<(u64, RoFlags), Error> { @@ -34,34 +41,34 @@ pub trait WasiFile: Send + Sync { } async fn sock_send<'a>( - &mut self, + &self, _si_data: &[std::io::IoSlice<'a>], _si_flags: SiFlags, ) -> Result { Err(Error::badf()) } - async fn sock_shutdown(&mut self, _how: SdFlags) -> Result<(), Error> { + async fn sock_shutdown(&self, _how: SdFlags) -> Result<(), Error> { Err(Error::badf()) } - async fn datasync(&mut self) -> Result<(), Error> { + async fn datasync(&self) -> Result<(), Error> { Ok(()) } - async fn sync(&mut self) -> Result<(), Error> { + async fn sync(&self) -> Result<(), Error> { Ok(()) } - async fn get_fdflags(&mut self) -> Result { + async fn get_fdflags(&self) -> Result { Ok(FdFlags::empty()) } - async fn set_fdflags(&mut self, _flags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&self, _flags: FdFlags) -> Result<(), Error> { Err(Error::badf()) } - async fn get_filestat(&mut self) -> Result { + async fn get_filestat(&self) -> Result { Ok(Filestat { device_id: 0, inode: 0, @@ -74,62 +81,59 @@ pub trait WasiFile: Send + Sync { }) } - async fn set_filestat_size(&mut self, _size: u64) -> Result<(), Error> { + async fn set_filestat_size(&self, _size: u64) -> Result<(), Error> { Err(Error::badf()) } - async fn advise(&mut self, _offset: u64, _len: u64, _advice: Advice) -> Result<(), Error> { + async fn advise(&self, _offset: u64, _len: u64, _advice: Advice) -> Result<(), Error> { Err(Error::badf()) } - async fn allocate(&mut self, _offset: u64, _len: u64) -> Result<(), Error> { + async fn allocate(&self, _offset: u64, _len: u64) -> Result<(), Error> { Err(Error::badf()) } async fn set_times( - &mut self, + &self, _atime: Option, _mtime: Option, ) -> Result<(), Error> { Err(Error::badf()) } - async fn read_vectored<'a>( - &mut self, - _bufs: &mut [std::io::IoSliceMut<'a>], - ) -> Result { + async fn read_vectored<'a>(&self, _bufs: &mut [std::io::IoSliceMut<'a>]) -> Result { Err(Error::badf()) } async fn read_vectored_at<'a>( - &mut self, + &self, _bufs: &mut [std::io::IoSliceMut<'a>], _offset: u64, ) -> Result { Err(Error::badf()) } - async fn write_vectored<'a>(&mut self, _bufs: &[std::io::IoSlice<'a>]) -> Result { + async fn write_vectored<'a>(&self, _bufs: &[std::io::IoSlice<'a>]) -> Result { Err(Error::badf()) } async fn write_vectored_at<'a>( - &mut self, + &self, _bufs: &[std::io::IoSlice<'a>], _offset: u64, ) -> Result { Err(Error::badf()) } - async fn seek(&mut self, _pos: std::io::SeekFrom) -> Result { + async fn seek(&self, _pos: std::io::SeekFrom) -> Result { Err(Error::badf()) } - async fn peek(&mut self, _buf: &mut [u8]) -> Result { + async fn peek(&self, _buf: &mut [u8]) -> Result { Err(Error::badf()) } - async fn num_ready_bytes(&self) -> Result { + fn num_ready_bytes(&self) -> Result { Ok(0) } @@ -212,33 +216,32 @@ pub struct Filestat { } pub(crate) trait TableFileExt { - fn get_file(&self, fd: u32) -> Result<&FileEntry, Error>; - fn get_file_mut(&mut self, fd: u32) -> Result<&mut FileEntry, Error>; + fn get_file(&self, fd: u32) -> Result, Error>; } impl TableFileExt for crate::table::Table { - fn get_file(&self, fd: u32) -> Result<&FileEntry, Error> { + fn get_file(&self, fd: u32) -> Result, Error> { self.get(fd) } - fn get_file_mut(&mut self, fd: u32) -> Result<&mut FileEntry, Error> { - self.get_mut(fd) - } } pub(crate) struct FileEntry { - caps: FileCaps, + caps: RwLock, file: Box, } impl FileEntry { pub fn new(caps: FileCaps, file: Box) -> Self { - FileEntry { caps, file } + FileEntry { + caps: RwLock::new(caps), + file, + } } pub fn capable_of(&self, caps: FileCaps) -> Result<(), Error> { - if self.caps.contains(caps) { + if self.caps.read().unwrap().contains(caps) { Ok(()) } else { - let missing = caps & !self.caps; + let missing = caps & !(*self.caps.read().unwrap()); let err = if missing.intersects(FileCaps::READ | FileCaps::WRITE) { // `EBADF` is a little surprising here because it's also used // for unknown-file-descriptor errors, but it's what POSIX uses @@ -251,16 +254,17 @@ impl FileEntry { } } - pub fn drop_caps_to(&mut self, caps: FileCaps) -> Result<(), Error> { + pub fn drop_caps_to(&self, caps: FileCaps) -> Result<(), Error> { self.capable_of(caps)?; - self.caps = caps; + *self.caps.write().unwrap() = caps; Ok(()) } - pub async fn get_fdstat(&mut self) -> Result { + pub async fn get_fdstat(&self) -> Result { + let caps = self.caps.read().unwrap().clone(); Ok(FdStat { filetype: self.file.get_filetype().await?, - caps: self.caps, + caps, flags: self.file.get_fdflags().await?, }) } @@ -268,7 +272,6 @@ impl FileEntry { pub trait FileEntryExt { fn get_cap(&self, caps: FileCaps) -> Result<&dyn WasiFile, Error>; - fn get_cap_mut(&mut self, caps: FileCaps) -> Result<&mut dyn WasiFile, Error>; } impl FileEntryExt for FileEntry { @@ -276,11 +279,6 @@ impl FileEntryExt for FileEntry { self.capable_of(caps)?; Ok(&*self.file) } - - fn get_cap_mut(&mut self, caps: FileCaps) -> Result<&mut dyn WasiFile, Error> { - self.capable_of(caps)?; - Ok(&mut *self.file) - } } bitflags! { @@ -317,3 +315,39 @@ pub enum Advice { DontNeed, NoReuse, } + +#[cfg(unix)] +pub struct BorrowedAsFd<'a, T: AsFd>(&'a T); + +#[cfg(unix)] +impl<'a, T: AsFd> BorrowedAsFd<'a, T> { + pub fn new(t: &'a T) -> Self { + BorrowedAsFd(t) + } +} + +#[cfg(unix)] +impl AsFd for BorrowedAsFd<'_, T> { + #[inline] + fn as_fd(&self) -> BorrowedFd { + self.0.as_fd() + } +} + +#[cfg(windows)] +pub struct BorrowedAsRawHandleOrSocket<'a, T: AsRawHandleOrSocket>(&'a T); + +#[cfg(windows)] +impl<'a, T: AsRawHandleOrSocket> BorrowedAsRawHandleOrSocket<'a, T> { + pub fn new(t: &'a T) -> Self { + BorrowedAsRawHandleOrSocket(t) + } +} + +#[cfg(windows)] +impl AsRawHandleOrSocket for BorrowedAsRawHandleOrSocket<'_, T> { + #[inline] + fn as_raw_handle_or_socket(&self) -> RawHandleOrSocket { + self.0.as_raw_handle_or_socket() + } +} diff --git a/crates/wasi-common/src/pipe.rs b/crates/wasi-common/src/pipe.rs index a5fceb80a1b1..1700131bd6cc 100644 --- a/crates/wasi-common/src/pipe.rs +++ b/crates/wasi-common/src/pipe.rs @@ -105,10 +105,10 @@ impl WasiFile for ReadPipe { fn as_any(&self) -> &dyn Any { self } - async fn get_filetype(&mut self) -> Result { + async fn get_filetype(&self) -> Result { Ok(FileType::Pipe) } - async fn read_vectored<'a>(&mut self, bufs: &mut [io::IoSliceMut<'a>]) -> Result { + async fn read_vectored<'a>(&self, bufs: &mut [io::IoSliceMut<'a>]) -> Result { let n = self.borrow().read_vectored(bufs)?; Ok(n.try_into()?) } @@ -189,13 +189,13 @@ impl WasiFile for WritePipe { fn as_any(&self) -> &dyn Any { self } - async fn get_filetype(&mut self) -> Result { + async fn get_filetype(&self) -> Result { Ok(FileType::Pipe) } - async fn get_fdflags(&mut self) -> Result { + async fn get_fdflags(&self) -> Result { Ok(FdFlags::APPEND) } - async fn write_vectored<'a>(&mut self, bufs: &[io::IoSlice<'a>]) -> Result { + async fn write_vectored<'a>(&self, bufs: &[io::IoSlice<'a>]) -> Result { let n = self.borrow().write_vectored(bufs)?; Ok(n.try_into()?) } diff --git a/crates/wasi-common/src/snapshots/preview_0.rs b/crates/wasi-common/src/snapshots/preview_0.rs index 763f685fcccf..e2a47223af7b 100644 --- a/crates/wasi-common/src/snapshots/preview_0.rs +++ b/crates/wasi-common/src/snapshots/preview_0.rs @@ -528,10 +528,8 @@ impl wasi_unstable::WasiUnstable for WasiCtx { fd: types::Fd, iovs: &types::IovecArray<'a>, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::READ)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::READ)?; let iovs: Vec> = iovs .iter() @@ -601,10 +599,8 @@ impl wasi_unstable::WasiUnstable for WasiCtx { iovs: &types::IovecArray<'a>, offset: types::Filesize, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::READ | FileCaps::SEEK)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::READ | FileCaps::SEEK)?; let iovs: Vec> = iovs .iter() @@ -675,10 +671,8 @@ impl wasi_unstable::WasiUnstable for WasiCtx { fd: types::Fd, ciovs: &types::CiovecArray<'a>, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::WRITE)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::WRITE)?; let guest_slices: Vec> = ciovs .iter() @@ -704,10 +698,8 @@ impl wasi_unstable::WasiUnstable for WasiCtx { ciovs: &types::CiovecArray<'a>, offset: types::Filesize, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::WRITE | FileCaps::SEEK)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::WRITE | FileCaps::SEEK)?; let guest_slices: Vec> = ciovs .iter() @@ -953,7 +945,7 @@ impl wasi_unstable::WasiUnstable for WasiCtx { } } - let table = &mut self.table; + let table = &self.table; let mut sub_fds: HashSet = HashSet::new(); // We need these refmuts to outlive Poll, which will hold the &mut dyn WasiFile inside let mut reads: Vec<(u32, Userdata)> = Vec::new(); @@ -1003,8 +995,8 @@ impl wasi_unstable::WasiUnstable for WasiCtx { sub_fds.insert(fd); } table - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::POLL_READWRITE)?; + .get_file(u32::from(fd))? + .get_cap(FileCaps::POLL_READWRITE)?; reads.push((u32::from(fd), sub.userdata.into())); } types::SubscriptionU::FdWrite(writesub) => { @@ -1016,8 +1008,8 @@ impl wasi_unstable::WasiUnstable for WasiCtx { sub_fds.insert(fd); } table - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::POLL_READWRITE)?; + .get_file(u32::from(fd))? + .get_cap(FileCaps::POLL_READWRITE)?; writes.push((u32::from(fd), sub.userdata.into())); } } diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index eac6d8544e5d..094270d51d3b 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -14,6 +14,7 @@ use cap_std::time::{Duration, SystemClock}; use std::convert::{TryFrom, TryInto}; use std::io::{IoSlice, IoSliceMut}; use std::ops::Deref; +use std::sync::Arc; use wiggle::GuestPtr; pub mod error; @@ -111,8 +112,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { advice: types::Advice, ) -> Result<(), Error> { self.table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::ADVISE)? + .get_file(u32::from(fd))? + .get_cap(FileCaps::ADVISE)? .advise(offset, len, advice.into()) .await?; Ok(()) @@ -125,8 +126,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { len: types::Filesize, ) -> Result<(), Error> { self.table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::ALLOCATE)? + .get_file(u32::from(fd))? + .get_cap(FileCaps::ALLOCATE)? .allocate(offset, len) .await?; Ok(()) @@ -142,15 +143,15 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { } // fd_close must close either a File or a Dir handle if table.is::(fd) { - let _ = table.delete(fd); + let _ = table.delete::(fd); } else if table.is::(fd) { // We cannot close preopened directories - let dir_entry: &DirEntry = table.get(fd).unwrap(); + let dir_entry: Arc = table.get(fd).unwrap(); if dir_entry.preopen_path().is_some() { return Err(Error::not_supported().context("cannot close propened directory")); } drop(dir_entry); - let _ = table.delete(fd); + let _ = table.delete::(fd); } else { return Err(Error::badf().context("key does not refer to file or directory")); } @@ -160,8 +161,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { async fn fd_datasync(&mut self, fd: types::Fd) -> Result<(), Error> { self.table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::DATASYNC)? + .get_file(u32::from(fd))? + .get_cap(FileCaps::DATASYNC)? .datasync() .await?; Ok(()) @@ -171,11 +172,11 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { let table = self.table(); let fd = u32::from(fd); if table.is::(fd) { - let file_entry: &mut FileEntry = table.get_mut(fd)?; + let file_entry: Arc = table.get(fd)?; let fdstat = file_entry.get_fdstat().await?; Ok(types::Fdstat::from(&fdstat)) } else if table.is::(fd) { - let dir_entry: &DirEntry = table.get(fd)?; + let dir_entry: Arc = table.get(fd)?; let dir_fdstat = dir_entry.get_dir_fdstat(); Ok(types::Fdstat::from(&dir_fdstat)) } else { @@ -189,8 +190,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { flags: types::Fdflags, ) -> Result<(), Error> { self.table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::FDSTAT_SET_FLAGS)? + .get_file(u32::from(fd))? + .get_cap(FileCaps::FDSTAT_SET_FLAGS)? .set_fdflags(FdFlags::from(flags)) .await } @@ -204,11 +205,11 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { let table = self.table(); let fd = u32::from(fd); if table.is::(fd) { - let file_entry: &mut FileEntry = table.get_mut(fd)?; + let file_entry: Arc = table.get(fd)?; let file_caps = FileCaps::from(&fs_rights_base); file_entry.drop_caps_to(file_caps) } else if table.is::(fd) { - let dir_entry: &mut DirEntry = table.get_mut(fd)?; + let dir_entry: Arc = table.get(fd)?; let dir_caps = DirCaps::from(&fs_rights_base); let file_caps = FileCaps::from(&fs_rights_inheriting); dir_entry.drop_caps_to(dir_caps, file_caps) @@ -222,8 +223,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { let fd = u32::from(fd); if table.is::(fd) { let filestat = table - .get_file_mut(fd)? - .get_cap_mut(FileCaps::FILESTAT_GET)? + .get_file(fd)? + .get_cap(FileCaps::FILESTAT_GET)? .get_filestat() .await?; Ok(filestat.into()) @@ -245,8 +246,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { size: types::Filesize, ) -> Result<(), Error> { self.table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::FILESTAT_SET_SIZE)? + .get_file(u32::from(fd))? + .get_cap(FileCaps::FILESTAT_SET_SIZE)? .set_filestat_size(size) .await?; Ok(()) @@ -272,9 +273,9 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { if table.is::(fd) { table - .get_file_mut(fd) + .get_file(fd) .expect("checked that entry is file") - .get_cap_mut(FileCaps::FILESTAT_SET_TIMES)? + .get_cap(FileCaps::FILESTAT_SET_TIMES)? .set_times(atim, mtim) .await } else if table.is::(fd) { @@ -294,10 +295,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { fd: types::Fd, iovs: &types::IovecArray<'a>, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::READ)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::READ)?; let iovs: Vec> = iovs .iter() @@ -367,10 +366,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { iovs: &types::IovecArray<'a>, offset: types::Filesize, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::READ | FileCaps::SEEK)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::READ | FileCaps::SEEK)?; let iovs: Vec> = iovs .iter() @@ -441,10 +438,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { fd: types::Fd, ciovs: &types::CiovecArray<'a>, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::WRITE)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::WRITE)?; let guest_slices: Vec> = ciovs .iter() @@ -470,10 +465,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { ciovs: &types::CiovecArray<'a>, offset: types::Filesize, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::WRITE | FileCaps::SEEK)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::WRITE | FileCaps::SEEK)?; let guest_slices: Vec> = ciovs .iter() @@ -495,7 +488,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { async fn fd_prestat_get(&mut self, fd: types::Fd) -> Result { let table = self.table(); - let dir_entry: &DirEntry = table.get(u32::from(fd)).map_err(|_| Error::badf())?; + let dir_entry: Arc = table.get(u32::from(fd)).map_err(|_| Error::badf())?; if let Some(ref preopen) = dir_entry.preopen_path() { let path_str = preopen.to_str().ok_or_else(|| Error::not_supported())?; let pr_name_len = u32::try_from(path_str.as_bytes().len())?; @@ -512,7 +505,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { path_max_len: types::Size, ) -> Result<(), Error> { let table = self.table(); - let dir_entry: &DirEntry = table.get(u32::from(fd)).map_err(|_| Error::not_dir())?; + let dir_entry: Arc = table.get(u32::from(fd)).map_err(|_| Error::not_dir())?; if let Some(ref preopen) = dir_entry.preopen_path() { let path_bytes = preopen .to_str() @@ -538,11 +531,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { if table.is_preopen(from) || table.is_preopen(to) { return Err(Error::not_supported().context("cannot renumber a preopen")); } - let from_entry = table - .delete(from) - .expect("we checked that table contains from"); - table.insert_at(to, from_entry); - Ok(()) + table.renumber(from, to) } async fn fd_seek( @@ -566,8 +555,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { }; let newoffset = self .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(required_caps)? + .get_file(u32::from(fd))? + .get_cap(required_caps)? .seek(whence) .await?; Ok(newoffset) @@ -575,8 +564,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { async fn fd_sync(&mut self, fd: types::Fd) -> Result<(), Error> { self.table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::SYNC)? + .get_file(u32::from(fd))? + .get_cap(FileCaps::SYNC)? .sync() .await?; Ok(()) @@ -586,8 +575,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { // XXX should this be stream_position? let offset = self .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::TELL)? + .get_file(u32::from(fd))? + .get_cap(FileCaps::TELL)? .seek(std::io::SeekFrom::Current(0)) .await?; Ok(offset) @@ -714,12 +703,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { target_path: &GuestPtr<'a, str>, ) -> Result<(), Error> { let table = self.table(); - let src_dir = table - .get_dir(u32::from(src_fd))? - .get_cap(DirCaps::LINK_SOURCE)?; - let target_dir = table - .get_dir(u32::from(target_fd))? - .get_cap(DirCaps::LINK_TARGET)?; + let src_dir = table.get_dir(u32::from(src_fd))?; + let src_dir = src_dir.get_cap(DirCaps::LINK_SOURCE)?; + let target_dir = table.get_dir(u32::from(target_fd))?; + let target_dir = target_dir.get_cap(DirCaps::LINK_TARGET)?; let symlink_follow = src_flags.contains(types::Lookupflags::SYMLINK_FOLLOW); if symlink_follow { return Err(Error::invalid_argument() @@ -769,7 +756,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { let dir = dir_entry.get_cap(DirCaps::OPEN)?; let child_dir = dir.open_dir(symlink_follow, path.deref()).await?; drop(dir); - let fd = table.push(Box::new(DirEntry::new( + let fd = table.push(Arc::new(DirEntry::new( dir_caps, file_caps, None, child_dir, )))?; Ok(types::Fd::from(fd)) @@ -789,7 +776,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .open_file(symlink_follow, path.deref(), oflags, read, write, fdflags) .await?; drop(dir); - let fd = table.push(Box::new(FileEntry::new(file_caps, file)))?; + let fd = table.push(Arc::new(FileEntry::new(file_caps, file)))?; Ok(types::Fd::from(fd)) } } @@ -839,12 +826,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { dest_path: &GuestPtr<'a, str>, ) -> Result<(), Error> { let table = self.table(); - let src_dir = table - .get_dir(u32::from(src_fd))? - .get_cap(DirCaps::RENAME_SOURCE)?; - let dest_dir = table - .get_dir(u32::from(dest_fd))? - .get_cap(DirCaps::RENAME_TARGET)?; + let src_dir = table.get_dir(u32::from(src_fd))?; + let src_dir = src_dir.get_cap(DirCaps::RENAME_SOURCE)?; + let dest_dir = table.get_dir(u32::from(dest_fd))?; + let dest_dir = dest_dir.get_cap(DirCaps::RENAME_TARGET)?; src_dir .rename( src_path.as_cow()?.deref(), @@ -914,10 +899,11 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { } } - let table = &mut self.table; + let table = &self.table; // We need these refmuts to outlive Poll, which will hold the &mut dyn WasiFile inside - let mut read_refs: Vec<(&dyn WasiFile, Userdata)> = Vec::new(); - let mut write_refs: Vec<(&dyn WasiFile, Userdata)> = Vec::new(); + let mut read_refs: Vec<(Arc, Option)> = Vec::new(); + let mut write_refs: Vec<(Arc, Option)> = Vec::new(); + let mut poll = Poll::new(); let subs = subs.as_array(nsubscriptions); @@ -983,25 +969,37 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { }, types::SubscriptionU::FdRead(readsub) => { let fd = readsub.file_descriptor; - let file_ref = table - .get_file(u32::from(fd))? - .get_cap(FileCaps::POLL_READWRITE)?; - read_refs.push((file_ref, sub.userdata.into())); + let file_ref = table.get_file(u32::from(fd))?; + let _file = file_ref.get_cap(FileCaps::POLL_READWRITE)?; + + read_refs.push((file_ref, Some(sub.userdata.into()))); } types::SubscriptionU::FdWrite(writesub) => { let fd = writesub.file_descriptor; - let file_ref = table - .get_file(u32::from(fd))? - .get_cap(FileCaps::POLL_READWRITE)?; - write_refs.push((file_ref, sub.userdata.into())); + let file_ref = table.get_file(u32::from(fd))?; + let _file = file_ref.get_cap(FileCaps::POLL_READWRITE)?; + write_refs.push((file_ref, Some(sub.userdata.into()))); } } } - for (f, ud) in read_refs.iter_mut() { + let mut read_mut_refs: Vec<(&dyn WasiFile, Userdata)> = Vec::new(); + for (file_lock, userdata) in read_refs.iter_mut() { + let file = file_lock.get_cap(FileCaps::POLL_READWRITE)?; + read_mut_refs.push((file, userdata.take().unwrap())); + } + + for (f, ud) in read_mut_refs.iter_mut() { poll.subscribe_read(*f, *ud); } - for (f, ud) in write_refs.iter_mut() { + + let mut write_mut_refs: Vec<(&dyn WasiFile, Userdata)> = Vec::new(); + for (file_lock, userdata) in write_refs.iter_mut() { + let file = file_lock.get_cap(FileCaps::POLL_READWRITE)?; + write_mut_refs.push((file, userdata.take().unwrap())); + } + + for (f, ud) in write_mut_refs.iter_mut() { poll.subscribe_write(*f, *ud); } @@ -1135,9 +1133,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { flags: types::Fdflags, ) -> Result { let table = self.table(); - let f = table - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::READ)?; + let f = table.get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::READ)?; let file = f.sock_accept(FdFlags::from(flags)).await?; let file_caps = FileCaps::READ @@ -1146,7 +1143,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { | FileCaps::POLL_READWRITE | FileCaps::FILESTAT_GET; - let fd = table.push(Box::new(FileEntry::new(file_caps, file)))?; + let fd = table.push(Arc::new(FileEntry::new(file_caps, file)))?; Ok(types::Fd::from(fd)) } @@ -1156,10 +1153,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { ri_data: &types::IovecArray<'a>, ri_flags: types::Riflags, ) -> Result<(types::Size, types::Roflags), Error> { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::READ)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::READ)?; let iovs: Vec> = ri_data .iter() @@ -1231,10 +1226,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { si_data: &types::CiovecArray<'a>, _si_flags: types::Siflags, ) -> Result { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::WRITE)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::WRITE)?; let guest_slices: Vec> = si_data .iter() @@ -1255,10 +1248,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { } async fn sock_shutdown(&mut self, fd: types::Fd, how: types::Sdflags) -> Result<(), Error> { - let f = self - .table() - .get_file_mut(u32::from(fd))? - .get_cap_mut(FileCaps::FDSTAT_SET_FLAGS)?; + let f = self.table().get_file(u32::from(fd))?; + let f = f.get_cap(FileCaps::FDSTAT_SET_FLAGS)?; f.sock_shutdown(SdFlags::from(how)).await } diff --git a/crates/wasi-common/src/table.rs b/crates/wasi-common/src/table.rs index 195d8babb677..a92fefff6a2a 100644 --- a/crates/wasi-common/src/table.rs +++ b/crates/wasi-common/src/table.rs @@ -1,6 +1,7 @@ use crate::{Error, ErrorExt}; use std::any::Any; use std::collections::HashMap; +use std::sync::{Arc, RwLock}; /// The `Table` type is designed to map u32 handles to resources. The table is now part of the /// public interface to a `WasiCtx` - it is reference counted so that it can be shared beyond a @@ -9,84 +10,89 @@ use std::collections::HashMap; /// /// The `Table` type is intended to model how the Interface Types concept of Resources is shaping /// up. Right now it is just an approximation. -pub struct Table { - map: HashMap>, +pub struct Table(RwLock); + +struct Inner { + map: HashMap>, next_key: u32, } impl Table { /// Create an empty table. New insertions will begin at 3, above stdio. pub fn new() -> Self { - Table { + Table(RwLock::new(Inner { map: HashMap::new(), next_key: 3, // 0, 1 and 2 are reserved for stdio - } + })) } /// Insert a resource at a certain index. - pub fn insert_at(&mut self, key: u32, a: Box) { - self.map.insert(key, a); + pub fn insert_at(&self, key: u32, a: Arc) { + self.0.write().unwrap().map.insert(key, a); } /// Insert a resource at the next available index. - pub fn push(&mut self, a: Box) -> Result { + pub fn push(&self, a: Arc) -> Result { + let mut inner = self.0.write().unwrap(); // NOTE: The performance of this new key calculation could be very bad once keys wrap // around. - if self.map.len() == u32::MAX as usize { + if inner.map.len() == u32::MAX as usize { return Err(Error::trap(anyhow::Error::msg("table has no free keys"))); } loop { - let key = self.next_key; - self.next_key = self.next_key.wrapping_add(1); - if self.map.contains_key(&key) { + let key = inner.next_key; + inner.next_key += 1; + if inner.map.contains_key(&key) { continue; } - self.map.insert(key, a); + inner.map.insert(key, a); return Ok(key); } } /// Check if the table has a resource at the given index. pub fn contains_key(&self, key: u32) -> bool { - self.map.contains_key(&key) + self.0.read().unwrap().map.contains_key(&key) } /// Check if the resource at a given index can be downcast to a given type. /// Note: this will always fail if the resource is already borrowed. pub fn is(&self, key: u32) -> bool { - if let Some(r) = self.map.get(&key) { + if let Some(r) = self.0.read().unwrap().map.get(&key) { r.is::() } else { false } } - /// Get an immutable reference to a resource of a given type at a given index. Multiple - /// immutable references can be borrowed at any given time. Borrow failure - /// results in a trapping error. - pub fn get(&self, key: u32) -> Result<&T, Error> { - if let Some(r) = self.map.get(&key) { - r.downcast_ref::() - .ok_or_else(|| Error::badf().context("element is a different type")) + /// Get an Arc reference to a resource of a given type at a given index. Multiple + /// immutable references can be borrowed at any given time. + pub fn get(&self, key: u32) -> Result, Error> { + if let Some(r) = self.0.read().unwrap().map.get(&key).cloned() { + r.downcast::() + .map_err(|_| Error::badf().context("element is a different type")) } else { Err(Error::badf().context("key not in table")) } } - /// Get a mutable reference to a resource of a given type at a given index. Only one mutable - /// reference can be borrowed at any given time. Borrow failure results in a trapping error. - pub fn get_mut(&mut self, key: u32) -> Result<&mut T, Error> { - if let Some(r) = self.map.get_mut(&key) { - r.downcast_mut::() - .ok_or_else(|| Error::badf().context("element is a different type")) - } else { - Err(Error::badf().context("key not in table")) - } + /// Remove a resource at a given index from the table. Returns the resource + /// if it was present. + pub fn delete(&self, key: u32) -> Option> { + self.0 + .write() + .unwrap() + .map + .remove(&key) + .map(|r| r.downcast::().unwrap()) } /// Remove a resource at a given index from the table. Returns the resource /// if it was present. - pub fn delete(&mut self, key: u32) -> Option> { - self.map.remove(&key) + pub fn renumber(&self, from: u32, to: u32) -> Result<(), Error> { + let map = &mut self.0.write().unwrap().map; + let from_entry = map.remove(&from).ok_or(Error::badf())?; + map.insert(to, from_entry); + Ok(()) } } diff --git a/crates/wasi-common/tokio/src/file.rs b/crates/wasi-common/tokio/src/file.rs index 030e60e5119c..91e70aa58397 100644 --- a/crates/wasi-common/tokio/src/file.rs +++ b/crates/wasi-common/tokio/src/file.rs @@ -4,7 +4,9 @@ use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; #[cfg(not(windows))] use io_lifetimes::AsFd; use std::any::Any; +use std::borrow::Borrow; use std::io; +use std::sync::Arc; use wasi_common::{ file::{Advice, FdFlags, FileType, Filestat, WasiFile}, Error, @@ -95,81 +97,80 @@ macro_rules! wasi_file_impl { self } #[cfg(unix)] - fn pollable(&self) -> Option { - Some(self.0.as_fd()) + fn pollable(&self) -> Option> { + self.0.pollable() } - #[cfg(windows)] - fn pollable(&self) -> Option { - Some(self.0.as_raw_handle_or_socket()) + fn pollable(&self) -> Option> { + self.0.pollable() } - async fn datasync(&mut self) -> Result<(), Error> { + async fn datasync(&self) -> Result<(), Error> { block_on_dummy_executor(|| self.0.datasync()) } - async fn sync(&mut self) -> Result<(), Error> { + async fn sync(&self) -> Result<(), Error> { block_on_dummy_executor(|| self.0.sync()) } - async fn get_filetype(&mut self) -> Result { + async fn get_filetype(&self) -> Result { block_on_dummy_executor(|| self.0.get_filetype()) } - async fn get_fdflags(&mut self) -> Result { + async fn get_fdflags(&self) -> Result { block_on_dummy_executor(|| self.0.get_fdflags()) } - async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&self, fdflags: FdFlags) -> Result<(), Error> { block_on_dummy_executor(|| self.0.set_fdflags(fdflags)) } - async fn get_filestat(&mut self) -> Result { + async fn get_filestat(&self) -> Result { block_on_dummy_executor(|| self.0.get_filestat()) } - async fn set_filestat_size(&mut self, size: u64) -> Result<(), Error> { + async fn set_filestat_size(&self, size: u64) -> Result<(), Error> { block_on_dummy_executor(move || self.0.set_filestat_size(size)) } - async fn advise(&mut self, offset: u64, len: u64, advice: Advice) -> Result<(), Error> { + async fn advise(&self, offset: u64, len: u64, advice: Advice) -> Result<(), Error> { block_on_dummy_executor(move || self.0.advise(offset, len, advice)) } - async fn allocate(&mut self, offset: u64, len: u64) -> Result<(), Error> { + async fn allocate(&self, offset: u64, len: u64) -> Result<(), Error> { block_on_dummy_executor(move || self.0.allocate(offset, len)) } async fn read_vectored<'a>( - &mut self, + &self, bufs: &mut [io::IoSliceMut<'a>], ) -> Result { block_on_dummy_executor(move || self.0.read_vectored(bufs)) } async fn read_vectored_at<'a>( - &mut self, + &self, bufs: &mut [io::IoSliceMut<'a>], offset: u64, ) -> Result { block_on_dummy_executor(move || self.0.read_vectored_at(bufs, offset)) } - async fn write_vectored<'a>(&mut self, bufs: &[io::IoSlice<'a>]) -> Result { + async fn write_vectored<'a>(&self, bufs: &[io::IoSlice<'a>]) -> Result { block_on_dummy_executor(move || self.0.write_vectored(bufs)) } async fn write_vectored_at<'a>( - &mut self, + &self, bufs: &[io::IoSlice<'a>], offset: u64, ) -> Result { block_on_dummy_executor(move || self.0.write_vectored_at(bufs, offset)) } - async fn seek(&mut self, pos: std::io::SeekFrom) -> Result { + async fn seek(&self, pos: std::io::SeekFrom) -> Result { block_on_dummy_executor(move || self.0.seek(pos)) } - async fn peek(&mut self, buf: &mut [u8]) -> Result { + async fn peek(&self, buf: &mut [u8]) -> Result { block_on_dummy_executor(move || self.0.peek(buf)) } async fn set_times( - &mut self, + &self, atime: Option, mtime: Option, ) -> Result<(), Error> { block_on_dummy_executor(move || self.0.set_times(atime, mtime)) } - async fn num_ready_bytes(&self) -> Result { - block_on_dummy_executor(|| self.0.num_ready_bytes()) + fn num_ready_bytes(&self) -> Result { + self.0.num_ready_bytes() } - fn isatty(&mut self) -> bool { + fn isatty(&self) -> bool { self.0.isatty() } @@ -182,7 +183,7 @@ macro_rules! wasi_file_impl { // lifetime of the AsyncFd. use std::os::unix::io::AsRawFd; use tokio::io::{unix::AsyncFd, Interest}; - let rawfd = self.0.as_fd().as_raw_fd(); + let rawfd = self.0.borrow().as_fd().as_raw_fd(); match AsyncFd::with_interest(rawfd, Interest::READABLE) { Ok(asyncfd) => { let _ = asyncfd.readable().await?; @@ -206,7 +207,7 @@ macro_rules! wasi_file_impl { // lifetime of the AsyncFd. use std::os::unix::io::AsRawFd; use tokio::io::{unix::AsyncFd, Interest}; - let rawfd = self.0.as_fd().as_raw_fd(); + let rawfd = self.0.borrow().as_fd().as_raw_fd(); match AsyncFd::with_interest(rawfd, Interest::WRITABLE) { Ok(asyncfd) => { let _ = asyncfd.writable().await?; @@ -221,7 +222,7 @@ macro_rules! wasi_file_impl { } } - async fn sock_accept(&mut self, fdflags: FdFlags) -> Result, Error> { + async fn sock_accept(&self, fdflags: FdFlags) -> Result, Error> { block_on_dummy_executor(|| self.0.sock_accept(fdflags)) } } @@ -229,7 +230,7 @@ macro_rules! wasi_file_impl { impl AsRawHandleOrSocket for $ty { #[inline] fn as_raw_handle_or_socket(&self) -> RawHandleOrSocket { - self.0.as_raw_handle_or_socket() + self.0.borrow().as_raw_handle_or_socket() } } }; diff --git a/crates/wasi-common/tokio/src/lib.rs b/crates/wasi-common/tokio/src/lib.rs index 577c6e2e1e38..1c7a1decb300 100644 --- a/crates/wasi-common/tokio/src/lib.rs +++ b/crates/wasi-common/tokio/src/lib.rs @@ -62,15 +62,15 @@ impl WasiCtxBuilder { } Ok(self) } - pub fn stdin(mut self, f: Box) -> Self { + pub fn stdin(self, f: Box) -> Self { self.0.set_stdin(f); self } - pub fn stdout(mut self, f: Box) -> Self { + pub fn stdout(self, f: Box) -> Self { self.0.set_stdout(f); self } - pub fn stderr(mut self, f: Box) -> Self { + pub fn stderr(self, f: Box) -> Self { self.0.set_stderr(f); self } @@ -87,7 +87,7 @@ impl WasiCtxBuilder { self.inherit_stdin().inherit_stdout().inherit_stderr() } pub fn preopened_dir( - mut self, + self, dir: cap_std::fs::Dir, guest_path: impl AsRef, ) -> Result { @@ -95,7 +95,7 @@ impl WasiCtxBuilder { self.0.push_preopened_dir(dir, guest_path)?; Ok(self) } - pub fn preopened_socket(mut self, fd: u32, socket: impl Into) -> Result { + pub fn preopened_socket(self, fd: u32, socket: impl Into) -> Result { let socket: Socket = socket.into(); let file: Box = socket.into(); diff --git a/crates/wasi-common/tokio/src/sched/unix.rs b/crates/wasi-common/tokio/src/sched/unix.rs index 5ca3b1200d09..4fd47d1cb248 100644 --- a/crates/wasi-common/tokio/src/sched/unix.rs +++ b/crates/wasi-common/tokio/src/sched/unix.rs @@ -63,7 +63,6 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> { f.complete( f.file .num_ready_bytes() - .await .map_err(|e| e.context("read num_ready_bytes"))?, RwEventFlags::empty(), ); diff --git a/crates/wasi-common/tokio/tests/poll_oneoff.rs b/crates/wasi-common/tokio/tests/poll_oneoff.rs index abaacef891fc..9ba85f6deeb5 100644 --- a/crates/wasi-common/tokio/tests/poll_oneoff.rs +++ b/crates/wasi-common/tokio/tests/poll_oneoff.rs @@ -20,7 +20,7 @@ async fn empty_file_readable() -> Result<(), Error> { let d = workspace.open_dir("d").context("open dir")?; let d = Dir::from_cap_std(d); - let mut f = d + let f = d .open_file(false, "f", OFlags::CREATE, false, true, FdFlags::empty()) .await .context("create writable file f")?; From 0dd41b6886083a757eec1aacaf41fba111e3c008 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 20 Dec 2022 13:11:49 -0800 Subject: [PATCH 02/33] wasi-threads: add an initial implementation This change is a first step toward implementing `wasi-threads` in Wasmtime. We may find that it has some missing pieces, but the core functionality is there: when `wasi::thread_spawn` is called by a running WebAssembly module, a function named `wasi_thread_start` is found in the module's exports and called in a new instance. The shared memory of the original instance is reused in the new instance. This new WASI proposal is in its early stages and details are still being hashed out in the [spec] and [wasi-libc] repositories. Due to its experimental state, the `wasi-threads` functionality is hidden behind both a compile-time and runtime flag: one must build with `--features wasi-threads` but also run the Wasmtime CLI with `--wasm-features threads` and `--wasi-modules experimental-wasi-threads`. One can experiment with `wasi-threads` by running: ```console $ cargo run --features wasi-threads -- \ --wasm-features threads --wasi-modules experimental-wasi-threads \ ``` Threads-enabled Wasm modules are not yet easy to build. Hopefully this is resolved soon, but in the meantime see the use of `THREAD_MODEL=posix` in the [wasi-libc] repository for some clues on what is necessary. Wiggle complicates things by requiring the Wasm memory to be exported with a certain name and `wasi-threads` also expects that memory to be imported; this build-time obstacle can be overcome with the `--import-memory --export-memory` flags only available in the latest Clang tree. Due to all of this, the included tests are written directly in WAT--run these with: ```console $ cargo test --features wasi-threads -p wasmtime-cli -- cli_tests ``` [spec]: https://github.com/WebAssembly/wasi-threads [wasi-libc]: https://github.com/WebAssembly/wasi-libc This change does not protect the WASI implementations themselves from concurrent access. This is already complete in previous commits or left for future commits in certain cases (e.g., wasi-nn). --- Cargo.lock | 12 ++ Cargo.toml | 5 +- crates/cli-flags/src/lib.rs | 36 ++++-- crates/wasi-threads/Cargo.toml | 22 ++++ crates/wasi-threads/LICENSE | 220 ++++++++++++++++++++++++++++++++ crates/wasi-threads/README.md | 3 + crates/wasi-threads/src/lib.rs | 139 ++++++++++++++++++++ src/commands/run.rs | 114 ++++++++++++----- tests/all/cli_tests.rs | 20 +++ tests/all/cli_tests/threads.wat | 55 ++++++++ 10 files changed, 585 insertions(+), 41 deletions(-) create mode 100644 crates/wasi-threads/Cargo.toml create mode 100644 crates/wasi-threads/LICENSE create mode 100644 crates/wasi-threads/README.md create mode 100644 crates/wasi-threads/src/lib.rs create mode 100644 tests/all/cli_tests/threads.wat diff --git a/Cargo.lock b/Cargo.lock index 8641a916fc3e..a10370b33f14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3506,6 +3506,7 @@ dependencies = [ "wasmtime-wasi", "wasmtime-wasi-crypto", "wasmtime-wasi-nn", + "wasmtime-wasi-threads", "wasmtime-wast", "wast 52.0.2", "wat", @@ -3774,6 +3775,17 @@ dependencies = [ "wiggle", ] +[[package]] +name = "wasmtime-wasi-threads" +version = "7.0.0" +dependencies = [ + "anyhow", + "log", + "rand 0.8.5", + "wasi-common", + "wasmtime", +] + [[package]] name = "wasmtime-wast" version = "7.0.0" diff --git a/Cargo.toml b/Cargo.toml index 45cb7af61a12..86966cb415c2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ wasmtime-wast = { workspace = true } wasmtime-wasi = { workspace = true } wasmtime-wasi-crypto = { workspace = true, optional = true } wasmtime-wasi-nn = { workspace = true, optional = true } +wasmtime-wasi-threads = { workspace = true, optional = true } clap = { workspace = true, features = ["color", "suggestions", "derive"] } anyhow = { workspace = true } target-lexicon = { workspace = true } @@ -88,6 +89,7 @@ members = [ "crates/bench-api", "crates/c-api", "crates/cli-flags", + "crates/wasi-threads", "crates/environ/fuzz", "crates/jit-icache-coherence", "crates/winch", @@ -124,6 +126,7 @@ wasmtime-wast = { path = "crates/wast", version = "=7.0.0" } wasmtime-wasi = { path = "crates/wasi", version = "7.0.0" } wasmtime-wasi-crypto = { path = "crates/wasi-crypto", version = "7.0.0" } wasmtime-wasi-nn = { path = "crates/wasi-nn", version = "7.0.0" } +wasmtime-wasi-threads = { path = "crates/wasi-threads", version = "7.0.0" } wasmtime-component-util = { path = "crates/component-util", version = "=7.0.0" } wasmtime-component-macro = { path = "crates/component-macro", version = "=7.0.0" } wasmtime-asm-macros = { path = "crates/asm-macros", version = "=7.0.0" } @@ -198,13 +201,13 @@ default = [ "wasmtime/wat", "wasmtime/parallel-compilation", "vtune", - "wasi-nn", "pooling-allocator", ] jitdump = ["wasmtime/jitdump"] vtune = ["wasmtime/vtune"] wasi-crypto = ["dep:wasmtime-wasi-crypto"] wasi-nn = ["dep:wasmtime-wasi-nn"] +wasi-threads = ["dep:wasmtime-wasi-threads"] pooling-allocator = ["wasmtime/pooling-allocator", "wasmtime-cli-flags/pooling-allocator"] all-arch = ["wasmtime/all-arch"] posix-signals-on-macos = ["wasmtime/posix-signals-on-macos"] diff --git a/crates/cli-flags/src/lib.rs b/crates/cli-flags/src/lib.rs index fb778af2dea5..7e40bff3aa8d 100644 --- a/crates/cli-flags/src/lib.rs +++ b/crates/cli-flags/src/lib.rs @@ -50,13 +50,17 @@ pub const SUPPORTED_WASI_MODULES: &[(&str, &str)] = &[ "wasi-common", "enables support for the WASI common APIs, see https://github.com/WebAssembly/WASI", ), + ( + "experimental-wasi-crypto", + "enables support for the WASI cryptography APIs (experimental), see https://github.com/WebAssembly/wasi-crypto", + ), ( "experimental-wasi-nn", "enables support for the WASI neural network API (experimental), see https://github.com/WebAssembly/wasi-nn", ), ( - "experimental-wasi-crypto", - "enables support for the WASI cryptography APIs (experimental), see https://github.com/WebAssembly/wasi-crypto", + "experimental-wasi-threads", + "enables support for the WASI threading API (experimental), see https://github.com/WebAssembly/wasi-threads", ), ]; @@ -466,8 +470,9 @@ fn parse_wasi_modules(modules: &str) -> Result { let mut set = |module: &str, enable: bool| match module { "" => Ok(()), "wasi-common" => Ok(wasi_modules.wasi_common = enable), - "experimental-wasi-nn" => Ok(wasi_modules.wasi_nn = enable), "experimental-wasi-crypto" => Ok(wasi_modules.wasi_crypto = enable), + "experimental-wasi-nn" => Ok(wasi_modules.wasi_nn = enable), + "experimental-wasi-threads" => Ok(wasi_modules.wasi_threads = enable), "default" => bail!("'default' cannot be specified with other WASI modules"), _ => bail!("unsupported WASI module '{}'", module), }; @@ -494,19 +499,23 @@ pub struct WasiModules { /// parts once the implementation allows for it (e.g. wasi-fs, wasi-clocks, etc.). pub wasi_common: bool, - /// Enable the experimental wasi-nn implementation + /// Enable the experimental wasi-crypto implementation. + pub wasi_crypto: bool, + + /// Enable the experimental wasi-nn implementation. pub wasi_nn: bool, - /// Enable the experimental wasi-crypto implementation - pub wasi_crypto: bool, + /// Enable the experimental wasi-threads implementation. + pub wasi_threads: bool, } impl Default for WasiModules { fn default() -> Self { Self { wasi_common: true, - wasi_nn: false, wasi_crypto: false, + wasi_nn: false, + wasi_threads: false, } } } @@ -518,6 +527,7 @@ impl WasiModules { wasi_common: false, wasi_nn: false, wasi_crypto: false, + wasi_threads: false, } } } @@ -663,8 +673,9 @@ mod test { options.wasi_modules.unwrap(), WasiModules { wasi_common: true, + wasi_crypto: false, wasi_nn: false, - wasi_crypto: false + wasi_threads: false } ); } @@ -676,8 +687,9 @@ mod test { options.wasi_modules.unwrap(), WasiModules { wasi_common: true, + wasi_crypto: false, wasi_nn: false, - wasi_crypto: false + wasi_threads: false } ); } @@ -693,8 +705,9 @@ mod test { options.wasi_modules.unwrap(), WasiModules { wasi_common: false, + wasi_crypto: false, wasi_nn: true, - wasi_crypto: false + wasi_threads: false } ); } @@ -707,8 +720,9 @@ mod test { options.wasi_modules.unwrap(), WasiModules { wasi_common: false, + wasi_crypto: false, wasi_nn: false, - wasi_crypto: false + wasi_threads: false } ); } diff --git a/crates/wasi-threads/Cargo.toml b/crates/wasi-threads/Cargo.toml new file mode 100644 index 000000000000..6d212721464c --- /dev/null +++ b/crates/wasi-threads/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "wasmtime-wasi-threads" +version.workspace = true +authors.workspace = true +description = "Wasmtime implementation of the wasi-threads API" +documentation = "https://docs.rs/wasmtime-wasi-nn" +license = "Apache-2.0 WITH LLVM-exception" +categories = ["wasm", "parallelism", "threads"] +keywords = ["webassembly", "wasm", "neural-network"] +repository = "https://github.com/bytecodealliance/wasmtime" +readme = "README.md" +edition.workspace = true + +[dependencies] +anyhow = { workspace = true } +log = { workspace = true } +rand = "0.8" +wasi-common = { workspace = true } +wasmtime = { workspace = true } + +[badges] +maintenance = { status = "experimental" } diff --git a/crates/wasi-threads/LICENSE b/crates/wasi-threads/LICENSE new file mode 100644 index 000000000000..f9d81955f4bc --- /dev/null +++ b/crates/wasi-threads/LICENSE @@ -0,0 +1,220 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + + +--- LLVM Exceptions to the Apache 2.0 License ---- + +As an exception, if, as a result of your compiling your source code, portions +of this Software are embedded into an Object form of such source code, you +may redistribute such embedded portions in such Object form without complying +with the conditions of Sections 4(a), 4(b) and 4(d) of the License. + +In addition, if you combine or link compiled forms of this Software with +software that is licensed under the GPLv2 ("Combined Software") and if a +court of competent jurisdiction determines that the patent provision (Section +3), the indemnity provision (Section 9) or other Section of the License +conflicts with the conditions of the GPLv2, you may retroactively and +prospectively choose to deem waived or otherwise exclude such Section(s) of +the License, but only in their entirety and only with respect to the Combined +Software. + diff --git a/crates/wasi-threads/README.md b/crates/wasi-threads/README.md new file mode 100644 index 000000000000..f165ed6d4149 --- /dev/null +++ b/crates/wasi-threads/README.md @@ -0,0 +1,3 @@ +# wasmtime-wasi-threads + +TODO diff --git a/crates/wasi-threads/src/lib.rs b/crates/wasi-threads/src/lib.rs new file mode 100644 index 000000000000..b9a2055b0c61 --- /dev/null +++ b/crates/wasi-threads/src/lib.rs @@ -0,0 +1,139 @@ +//! Implement [`wasi-threads`]. +//! +//! [`wasi-threads`]: https://github.com/WebAssembly/wasi-threads + +use anyhow::{anyhow, Result}; +use rand::Rng; +use std::thread; +use wasi_common::I32Exit; +use wasmtime::{Caller, Linker, Module, SharedMemory, Store}; + +// This name is a function export designated by the wasi-threads specification: +// https://github.com/WebAssembly/wasi-threads/#detailed-design-discussion +const WASI_ENTRY_POINT: &str = "wasi_thread_start"; + +pub struct WasiThreadsCtx { + module: Module, + linker: Linker, +} + +impl WasiThreadsCtx { + pub fn new(module: Module, linker: Linker) -> Self { + Self { module, linker } + } + + pub fn spawn(&self, host: T, thread_start_arg: i32) -> Result { + let module = self.module.clone(); + let linker = self.linker.clone(); + + // Start a Rust thread running a new instance of the current module. + let wasi_thread_id = random_thread_id(); + let builder = thread::Builder::new().name(format!("wasi-thread-{}", wasi_thread_id)); + builder.spawn(move || { + // Convenience function for printing failures, since the `Thread` + // has no way to report a failure to the outer context. + let fail = |msg: String| { + format!( + "wasi-thread-{} exited unsuccessfully: {}", + wasi_thread_id, msg + ) + }; + + // Each new instance is created in its own store. + let mut store = Store::new(&module.engine(), host); + let instance = linker + .instantiate(&mut store, &module) + .expect(&fail("failed to instantiate".into())); + let thread_entry_point = instance + .get_typed_func::<(i32, i32), ()>(&mut store, WASI_ENTRY_POINT) + .expect(&fail(format!( + "failed to find wasi-threads entry point function: {}", + WASI_ENTRY_POINT + ))); + + // Start the thread's entry point; any failures are simply printed + // before exiting the thread. It may be necessary to handle failures + // here somehow, e.g., so that `pthread_join` can be notified if the + // user function traps for some reason (TODO). + log::trace!( + "spawned thread id = {}; calling start function `{}` with: {}", + wasi_thread_id, + WASI_ENTRY_POINT, + thread_start_arg + ); + match thread_entry_point.call(&mut store, (wasi_thread_id, thread_start_arg)) { + Ok(_) => {} + Err(trap) => { + if let Some(I32Exit(0)) = trap.downcast_ref::() { + log::trace!( + "exited thread id = {} via `wasi::proc_exit` call", + wasi_thread_id + ) + } else { + panic!("{}", fail(trap.to_string())) + } + } + } + })?; + + Ok(wasi_thread_id) + } +} + +/// Helper for generating valid WASI thread IDs (TID). +/// +/// Callers of `wasi_thread_spawn` expect a TID >=0 to indicate a successful +/// spawning of the thread whereas a negative return value indicates an +/// failure to spawn. +fn random_thread_id() -> i32 { + let tid: u32 = rand::thread_rng().gen(); + (tid >> 1) as i32 +} + +/// Manually add the WASI `thread_spawn` function to the linker. +/// +/// It is unclear what namespace the `wasi-threads` proposal should live under: +/// it is not clear if it should be included in any of the `preview*` releases +/// so for the time being its module namespace is simply `"wasi"` (TODO). +pub fn add_to_linker( + linker: &mut wasmtime::Linker, + store: &wasmtime::Store, + module: &Module, + get_cx: impl Fn(&mut T) -> &WasiThreadsCtx + Send + Sync + Copy + 'static, +) -> anyhow::Result { + linker.func_wrap( + "wasi", + "thread_spawn", + move |mut caller: Caller<'_, T>, start_arg: i32| -> i32 { + log::trace!("new thread requested via `wasi::thread_spawn` call"); + let host = caller.data().clone(); + let ctx = get_cx(caller.data_mut()); + match ctx.spawn(host, start_arg) { + Ok(thread_id) => { + assert!(thread_id >= 0, "thread_id = {}", thread_id); + thread_id + } + Err(e) => { + log::error!("failed to spawn thread: {}", e); + -1 + } + } + }, + )?; + + // Find the shared memory import and satisfy it with a newly-created shared + // memory import. This currently does not handle multiple memories (TODO). + for import in module.imports() { + if let Some(m) = import.ty().memory() { + if m.is_shared() { + let mem = SharedMemory::new(module.engine(), m.clone())?; + linker.define(store, import.module(), import.name(), mem.clone())?; + return Ok(mem); + } + } + } + Err(anyhow!( + "unable to link a shared memory import to the module; a `wasi-threads` \ + module should import a single shared memory as \"memory\"" + )) +} diff --git a/src/commands/run.rs b/src/commands/run.rs index 9b2ed8268a4e..5800a8b436d2 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -3,6 +3,7 @@ use anyhow::{anyhow, bail, Context as _, Result}; use clap::Parser; use once_cell::sync::Lazy; +use std::sync::Arc; use std::thread; use std::time::Duration; use std::{ @@ -21,6 +22,9 @@ use wasmtime_wasi_nn::WasiNnCtx; #[cfg(feature = "wasi-crypto")] use wasmtime_wasi_crypto::WasiCryptoCtx; +#[cfg(feature = "wasi-threads")] +use wasmtime_wasi_threads::WasiThreadsCtx; + fn parse_module(s: &OsStr) -> anyhow::Result { // Do not accept wasmtime subcommand names as the module name match s.to_str() { @@ -164,13 +168,6 @@ impl RunCommand { config.epoch_interruption(true); } let engine = Engine::new(&config)?; - let mut store = Store::new(&engine, Host::default()); - - // If fuel has been configured, we want to add the configured - // fuel amount to this store. - if let Some(fuel) = self.common.fuel { - store.add_fuel(fuel)?; - } let preopen_sockets = self.compute_preopen_sockets()?; @@ -181,9 +178,16 @@ impl RunCommand { let mut linker = Linker::new(&engine); linker.allow_unknown_exports(self.allow_unknown_exports); + // Read the wasm module binary either as `*.wat` or a raw binary. + let module = self.load_module(linker.engine(), &self.module)?; + + let mut host = Arc::new(Host::default()); + let mut store = Store::new(&engine, host.clone()); populate_with_wasi( - &mut store, + &mut host, &mut linker, + &store, + module.clone(), preopen_dirs, &argv, &self.vars, @@ -192,6 +196,12 @@ impl RunCommand { preopen_sockets, )?; + // If fuel has been configured, we want to add the configured + // fuel amount to this store. + if let Some(fuel) = self.common.fuel { + store.add_fuel(fuel)?; + } + // Load the preload wasm modules. for (name, path) in self.preloads.iter() { // Read the wasm module binary either as `*.wat` or a raw binary @@ -207,7 +217,7 @@ impl RunCommand { // Load the main wasm module. match self - .load_main_module(&mut store, &mut linker) + .load_main_module(&mut store, &mut linker, module) .with_context(|| format!("failed to run main module `{}`", self.module.display())) { Ok(()) => (), @@ -309,7 +319,12 @@ impl RunCommand { result } - fn load_main_module(&self, store: &mut Store, linker: &mut Linker) -> Result<()> { + fn load_main_module( + &self, + store: &mut Store>, + linker: &mut Linker>, + module: Module, + ) -> Result<()> { if let Some(timeout) = self.wasm_timeout { store.set_epoch_deadline(1); let engine = store.engine().clone(); @@ -319,8 +334,6 @@ impl RunCommand { }); } - // Read the wasm module binary either as `*.wat` or a raw binary. - let module = self.load_module(linker.engine(), &self.module)?; // The main module might be allowed to have unknown imports, which // should be defined as traps: if self.trap_unknown_imports { @@ -342,8 +355,8 @@ impl RunCommand { fn invoke_export( &self, - store: &mut Store, - linker: &Linker, + store: &mut Store>, + linker: &Linker>, name: &str, ) -> Result<()> { let func = match linker @@ -357,7 +370,12 @@ impl RunCommand { self.invoke_func(store, func, Some(name)) } - fn invoke_func(&self, store: &mut Store, func: Func, name: Option<&str>) -> Result<()> { + fn invoke_func( + &self, + store: &mut Store>, + func: Func, + name: Option<&str>, + ) -> Result<()> { let ty = func.ty(&store); if ty.params().len() > 0 { eprintln!( @@ -435,16 +453,20 @@ impl RunCommand { #[derive(Default)] struct Host { wasi: Option, - #[cfg(feature = "wasi-nn")] - wasi_nn: Option, #[cfg(feature = "wasi-crypto")] wasi_crypto: Option, + #[cfg(feature = "wasi-nn")] + wasi_nn: Option, + #[cfg(feature = "wasi-threads")] + wasi_threads: Option>>, } /// Populates the given `Linker` with WASI APIs. fn populate_with_wasi( - store: &mut Store, - linker: &mut Linker, + host: &mut Arc, + linker: &mut Linker>, + store: &Store>, + module: Module, preopen_dirs: Vec<(String, Dir)>, argv: &[String], vars: &[(String, String)], @@ -453,7 +475,9 @@ fn populate_with_wasi( mut tcplisten: Vec, ) -> Result<()> { if wasi_modules.wasi_common { - wasmtime_wasi::add_to_linker(linker, |host| host.wasi.as_mut().unwrap())?; + wasmtime_wasi::add_to_linker(linker, |host| { + Arc::get_mut(host).unwrap().wasi.as_mut().unwrap() + })?; let mut builder = WasiCtxBuilder::new(); builder = builder.inherit_stdio().args(argv)?.envs(vars)?; @@ -475,7 +499,28 @@ fn populate_with_wasi( builder = builder.preopened_dir(dir, name)?; } - store.data_mut().wasi = Some(builder.build()); + Arc::get_mut(host) + .expect("there must be no other host references during setup") + .wasi = Some(builder.build()); + } + + if wasi_modules.wasi_crypto { + #[cfg(not(feature = "wasi-crypto"))] + { + bail!("Cannot enable wasi-crypto when the binary is not compiled with this feature."); + } + #[cfg(feature = "wasi-crypto")] + { + wasmtime_wasi_crypto::add_to_linker(linker, |host| { + Arc::get_mut(host) + .wasi_crypto + .as_mut() + .expect("wasi-crypto is not implemented with multi-threading support") + })?; + Arc::get_mut(host) + .expect("there must be no other host references during setup") + .wasi_crypto = Some(WasiCryptoCtx::new()); + } } if wasi_modules.wasi_nn { @@ -485,20 +530,31 @@ fn populate_with_wasi( } #[cfg(feature = "wasi-nn")] { - wasmtime_wasi_nn::add_to_linker(linker, |host| host.wasi_nn.as_mut().unwrap())?; - store.data_mut().wasi_nn = Some(WasiNnCtx::new()?); + wasmtime_wasi_nn::add_to_linker(linker, |host| { + Arc::get_mut(host) + .wasi_nn + .as_mut() + .expect("wasi-nn is not implemented with multi-threading support") + })?; + Arc::get_mut(host) + .expect("there must be no other host references during setup") + .wasi_nn = Some(WasiNnCtx::new()?); } } - if wasi_modules.wasi_crypto { - #[cfg(not(feature = "wasi-crypto"))] + if wasi_modules.wasi_threads { + #[cfg(not(feature = "wasi-threads"))] { - bail!("Cannot enable wasi-crypto when the binary is not compiled with this feature."); + bail!("Cannot enable wasi-threads when the binary is not compiled with this feature."); } - #[cfg(feature = "wasi-crypto")] + #[cfg(feature = "wasi-threads")] { - wasmtime_wasi_crypto::add_to_linker(linker, |host| host.wasi_crypto.as_mut().unwrap())?; - store.data_mut().wasi_crypto = Some(WasiCryptoCtx::new()); + wasmtime_wasi_threads::add_to_linker(linker, store, &module, |host| { + host.wasi_threads.as_ref().unwrap() + })?; + Arc::get_mut(host) + .expect("there must be no other host references during setup") + .wasi_threads = Some(WasiThreadsCtx::new(module, linker.clone())); } } diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index b8dbfce83139..80bb92736180 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -473,3 +473,23 @@ fn run_cwasm_from_stdin() -> Result<()> { } Ok(()) } + +#[cfg(feature = "wasi-threads")] +#[test] +fn run_threads() -> Result<()> { + let wasm = build_wasm("tests/all/cli_tests/threads.wat")?; + let stdout = run_wasmtime(&[ + "run", + "--wasi-modules", + "experimental-wasi-threads", + "--wasm-features", + "threads", + "--disable-cache", + wasm.path().to_str().unwrap(), + ])?; + + assert!(stdout.contains("Hello _start")); + assert!(stdout.contains("Hello wasi_thread_start")); + assert!(stdout.contains("Hello done")); + Ok(()) +} diff --git a/tests/all/cli_tests/threads.wat b/tests/all/cli_tests/threads.wat new file mode 100644 index 000000000000..ca5847a60da8 --- /dev/null +++ b/tests/all/cli_tests/threads.wat @@ -0,0 +1,55 @@ +(module + ;; As we have discussed, it makes sense to make the shared memory an import + ;; so that all + (import "" "memory" (memory $shmem 1 1 shared)) + (import "wasi_snapshot_preview1" "fd_write" + (func $__wasi_fd_write (param i32 i32 i32 i32) (result i32))) + (import "wasi" "thread_spawn" + (func $__wasi_thread_spawn (param i32) (result i32))) + + (func (export "_start") + (local $i i32) + + ;; Print "Hello _start". + (call $print (i32.const 32) (i32.const 13)) + + ;; Print "Hello wasi_thread_start" in several threads. + (drop (call $__wasi_thread_spawn (i32.const 0))) + (drop (call $__wasi_thread_spawn (i32.const 0))) + (drop (call $__wasi_thread_spawn (i32.const 0))) + + ;; Wasmtime has no `wait/notify` yet, so we just spin to allow the threads + ;; to do their work. + (local.set $i (i32.const 2000000)) + (loop $again + (local.set $i (i32.sub (local.get $i) (i32.const 1))) + (br_if $again (i32.gt_s (local.get $i) (i32.const 0))) + ) + + ;; Print "Hello done". + (call $print (i32.const 64) (i32.const 11)) + ) + + ;; A threads-enabled module must export this spec-designated entry point. + (func (export "wasi_thread_start") (param $tid i32) (param $start_arg i32) + (call $print (i32.const 96) (i32.const 24)) + ) + + ;; A helper function for printing ptr-len strings. + (func $print (param $ptr i32) (param $len i32) + (i32.store (i32.const 8) (local.get $len)) + (i32.store (i32.const 4) (local.get $ptr)) + (drop (call $__wasi_fd_write + (i32.const 1) + (i32.const 4) + (i32.const 1) + (i32.const 0))) + ) + + ;; We still need to export the shared memory for Wiggle's sake. + (export "memory" (memory $shmem)) + + (data (i32.const 32) "Hello _start\0a") + (data (i32.const 64) "Hello done\0a") + (data (i32.const 96) "Hello wasi_thread_start\0a") +) From 8c07438c0b7171dc9392838ea71742bca4674a66 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Fri, 6 Jan 2023 15:38:50 -0800 Subject: [PATCH 03/33] review: remove LICENSE and README.md --- crates/wasi-threads/LICENSE | 220 ---------------------------------- crates/wasi-threads/README.md | 3 - 2 files changed, 223 deletions(-) delete mode 100644 crates/wasi-threads/LICENSE delete mode 100644 crates/wasi-threads/README.md diff --git a/crates/wasi-threads/LICENSE b/crates/wasi-threads/LICENSE deleted file mode 100644 index f9d81955f4bc..000000000000 --- a/crates/wasi-threads/LICENSE +++ /dev/null @@ -1,220 +0,0 @@ - - Apache License - Version 2.0, January 2004 - http://www.apache.org/licenses/ - - TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION - - 1. Definitions. - - "License" shall mean the terms and conditions for use, reproduction, - and distribution as defined by Sections 1 through 9 of this document. - - "Licensor" shall mean the copyright owner or entity authorized by - the copyright owner that is granting the License. - - "Legal Entity" shall mean the union of the acting entity and all - other entities that control, are controlled by, or are under common - control with that entity. For the purposes of this definition, - "control" means (i) the power, direct or indirect, to cause the - direction or management of such entity, whether by contract or - otherwise, or (ii) ownership of fifty percent (50%) or more of the - outstanding shares, or (iii) beneficial ownership of such entity. - - "You" (or "Your") shall mean an individual or Legal Entity - exercising permissions granted by this License. - - "Source" form shall mean the preferred form for making modifications, - including but not limited to software source code, documentation - source, and configuration files. - - "Object" form shall mean any form resulting from mechanical - transformation or translation of a Source form, including but - not limited to compiled object code, generated documentation, - and conversions to other media types. - - "Work" shall mean the work of authorship, whether in Source or - Object form, made available under the License, as indicated by a - copyright notice that is included in or attached to the work - (an example is provided in the Appendix below). - - "Derivative Works" shall mean any work, whether in Source or Object - form, that is based on (or derived from) the Work and for which the - editorial revisions, annotations, elaborations, or other modifications - represent, as a whole, an original work of authorship. For the purposes - of this License, Derivative Works shall not include works that remain - separable from, or merely link (or bind by name) to the interfaces of, - the Work and Derivative Works thereof. - - "Contribution" shall mean any work of authorship, including - the original version of the Work and any modifications or additions - to that Work or Derivative Works thereof, that is intentionally - submitted to Licensor for inclusion in the Work by the copyright owner - or by an individual or Legal Entity authorized to submit on behalf of - the copyright owner. For the purposes of this definition, "submitted" - means any form of electronic, verbal, or written communication sent - to the Licensor or its representatives, including but not limited to - communication on electronic mailing lists, source code control systems, - and issue tracking systems that are managed by, or on behalf of, the - Licensor for the purpose of discussing and improving the Work, but - excluding communication that is conspicuously marked or otherwise - designated in writing by the copyright owner as "Not a Contribution." - - "Contributor" shall mean Licensor and any individual or Legal Entity - on behalf of whom a Contribution has been received by Licensor and - subsequently incorporated within the Work. - - 2. Grant of Copyright License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - copyright license to reproduce, prepare Derivative Works of, - publicly display, publicly perform, sublicense, and distribute the - Work and such Derivative Works in Source or Object form. - - 3. Grant of Patent License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - (except as stated in this section) patent license to make, have made, - use, offer to sell, sell, import, and otherwise transfer the Work, - where such license applies only to those patent claims licensable - by such Contributor that are necessarily infringed by their - Contribution(s) alone or by combination of their Contribution(s) - with the Work to which such Contribution(s) was submitted. If You - institute patent litigation against any entity (including a - cross-claim or counterclaim in a lawsuit) alleging that the Work - or a Contribution incorporated within the Work constitutes direct - or contributory patent infringement, then any patent licenses - granted to You under this License for that Work shall terminate - as of the date such litigation is filed. - - 4. Redistribution. You may reproduce and distribute copies of the - Work or Derivative Works thereof in any medium, with or without - modifications, and in Source or Object form, provided that You - meet the following conditions: - - (a) You must give any other recipients of the Work or - Derivative Works a copy of this License; and - - (b) You must cause any modified files to carry prominent notices - stating that You changed the files; and - - (c) You must retain, in the Source form of any Derivative Works - that You distribute, all copyright, patent, trademark, and - attribution notices from the Source form of the Work, - excluding those notices that do not pertain to any part of - the Derivative Works; and - - (d) If the Work includes a "NOTICE" text file as part of its - distribution, then any Derivative Works that You distribute must - include a readable copy of the attribution notices contained - within such NOTICE file, excluding those notices that do not - pertain to any part of the Derivative Works, in at least one - of the following places: within a NOTICE text file distributed - as part of the Derivative Works; within the Source form or - documentation, if provided along with the Derivative Works; or, - within a display generated by the Derivative Works, if and - wherever such third-party notices normally appear. The contents - of the NOTICE file are for informational purposes only and - do not modify the License. You may add Your own attribution - notices within Derivative Works that You distribute, alongside - or as an addendum to the NOTICE text from the Work, provided - that such additional attribution notices cannot be construed - as modifying the License. - - You may add Your own copyright statement to Your modifications and - may provide additional or different license terms and conditions - for use, reproduction, or distribution of Your modifications, or - for any such Derivative Works as a whole, provided Your use, - reproduction, and distribution of the Work otherwise complies with - the conditions stated in this License. - - 5. Submission of Contributions. Unless You explicitly state otherwise, - any Contribution intentionally submitted for inclusion in the Work - by You to the Licensor shall be under the terms and conditions of - this License, without any additional terms or conditions. - Notwithstanding the above, nothing herein shall supersede or modify - the terms of any separate license agreement you may have executed - with Licensor regarding such Contributions. - - 6. Trademarks. This License does not grant permission to use the trade - names, trademarks, service marks, or product names of the Licensor, - except as required for reasonable and customary use in describing the - origin of the Work and reproducing the content of the NOTICE file. - - 7. Disclaimer of Warranty. Unless required by applicable law or - agreed to in writing, Licensor provides the Work (and each - Contributor provides its Contributions) on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - implied, including, without limitation, any warranties or conditions - of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A - PARTICULAR PURPOSE. You are solely responsible for determining the - appropriateness of using or redistributing the Work and assume any - risks associated with Your exercise of permissions under this License. - - 8. Limitation of Liability. In no event and under no legal theory, - whether in tort (including negligence), contract, or otherwise, - unless required by applicable law (such as deliberate and grossly - negligent acts) or agreed to in writing, shall any Contributor be - liable to You for damages, including any direct, indirect, special, - incidental, or consequential damages of any character arising as a - result of this License or out of the use or inability to use the - Work (including but not limited to damages for loss of goodwill, - work stoppage, computer failure or malfunction, or any and all - other commercial damages or losses), even if such Contributor - has been advised of the possibility of such damages. - - 9. Accepting Warranty or Additional Liability. While redistributing - the Work or Derivative Works thereof, You may choose to offer, - and charge a fee for, acceptance of support, warranty, indemnity, - or other liability obligations and/or rights consistent with this - License. However, in accepting such obligations, You may act only - on Your own behalf and on Your sole responsibility, not on behalf - of any other Contributor, and only if You agree to indemnify, - defend, and hold each Contributor harmless for any liability - incurred by, or claims asserted against, such Contributor by reason - of your accepting any such warranty or additional liability. - - END OF TERMS AND CONDITIONS - - APPENDIX: How to apply the Apache License to your work. - - To apply the Apache License to your work, attach the following - boilerplate notice, with the fields enclosed by brackets "[]" - replaced with your own identifying information. (Don't include - the brackets!) The text should be enclosed in the appropriate - comment syntax for the file format. We also recommend that a - file or class name and description of purpose be included on the - same "printed page" as the copyright notice for easier - identification within third-party archives. - - Copyright [yyyy] [name of copyright owner] - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - - ---- LLVM Exceptions to the Apache 2.0 License ---- - -As an exception, if, as a result of your compiling your source code, portions -of this Software are embedded into an Object form of such source code, you -may redistribute such embedded portions in such Object form without complying -with the conditions of Sections 4(a), 4(b) and 4(d) of the License. - -In addition, if you combine or link compiled forms of this Software with -software that is licensed under the GPLv2 ("Combined Software") and if a -court of competent jurisdiction determines that the patent provision (Section -3), the indemnity provision (Section 9) or other Section of the License -conflicts with the conditions of the GPLv2, you may retroactively and -prospectively choose to deem waived or otherwise exclude such Section(s) of -the License, but only in their entirety and only with respect to the Combined -Software. - diff --git a/crates/wasi-threads/README.md b/crates/wasi-threads/README.md deleted file mode 100644 index f165ed6d4149..000000000000 --- a/crates/wasi-threads/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# wasmtime-wasi-threads - -TODO From f17e05b5723d54b1ec5e23f4d644117dbadac7a4 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 9 Jan 2023 11:39:02 -0800 Subject: [PATCH 04/33] wasi-threads: factor out process exit logic As is being discussed [elsewhere], either calling `proc_exit` or trapping in any thread should halt execution of all threads. The Wasmtime CLI already has logic for adapting a WebAssembly error code to a code expected in each OS. This change factors out this logic to a new function, `maybe_exit_on_error`, for use within the `wasi-threads` implementation. This will work reasonably well for CLI users of Wasmtime + `wasi-threads`, but embedders will want something better in the future: when a `wasi-threads` threads fails, they may not want their application to exit. Handling this is tricky, because it will require cancelling the threads spawned by the `wasi-threads` implementation, something that is not trivial to do in Rust. With this change, we defer that work until later in order to provide a working implementation of `wasi-threads` for experimentation. [elsewhere]: https://github.com/WebAssembly/wasi-threads/pull/17 --- Cargo.lock | 2 ++ Cargo.toml | 2 +- crates/wasi-threads/Cargo.toml | 1 + crates/wasi-threads/src/lib.rs | 16 +++++------- crates/wasi/Cargo.toml | 1 + crates/wasi/src/lib.rs | 42 +++++++++++++++++++++++++++++++ src/commands/run.rs | 45 ++++++---------------------------- 7 files changed, 60 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a10370b33f14..f2e2b6d4d43f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3747,6 +3747,7 @@ name = "wasmtime-wasi" version = "7.0.0" dependencies = [ "anyhow", + "libc", "wasi-cap-std-sync", "wasi-common", "wasi-tokio", @@ -3784,6 +3785,7 @@ dependencies = [ "rand 0.8.5", "wasi-common", "wasmtime", + "wasmtime-wasi", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 86966cb415c2..42d76b246130 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,6 @@ wasmtime-wasi-threads = { workspace = true, optional = true } clap = { workspace = true, features = ["color", "suggestions", "derive"] } anyhow = { workspace = true } target-lexicon = { workspace = true } -libc = "0.2.60" humantime = "2.0.0" once_cell = { workspace = true } listenfd = "1.0.0" @@ -69,6 +68,7 @@ wasmtime-component-util = { workspace = true } component-macro-test = { path = "crates/misc/component-macro-test" } component-test-util = { workspace = true } bstr = "0.2.17" +libc = "0.2.60" [target.'cfg(windows)'.dev-dependencies] windows-sys = { workspace = true, features = ["Win32_System_Memory"] } diff --git a/crates/wasi-threads/Cargo.toml b/crates/wasi-threads/Cargo.toml index 6d212721464c..2667587b83df 100644 --- a/crates/wasi-threads/Cargo.toml +++ b/crates/wasi-threads/Cargo.toml @@ -17,6 +17,7 @@ log = { workspace = true } rand = "0.8" wasi-common = { workspace = true } wasmtime = { workspace = true } +wasmtime-wasi = { workspace = true } [badges] maintenance = { status = "experimental" } diff --git a/crates/wasi-threads/src/lib.rs b/crates/wasi-threads/src/lib.rs index b9a2055b0c61..ebf0ac51788c 100644 --- a/crates/wasi-threads/src/lib.rs +++ b/crates/wasi-threads/src/lib.rs @@ -5,8 +5,8 @@ use anyhow::{anyhow, Result}; use rand::Rng; use std::thread; -use wasi_common::I32Exit; use wasmtime::{Caller, Linker, Module, SharedMemory, Store}; +use wasmtime_wasi::maybe_exit_on_error; // This name is a function export designated by the wasi-threads specification: // https://github.com/WebAssembly/wasi-threads/#detailed-design-discussion @@ -63,15 +63,11 @@ impl WasiThreadsCtx { ); match thread_entry_point.call(&mut store, (wasi_thread_id, thread_start_arg)) { Ok(_) => {} - Err(trap) => { - if let Some(I32Exit(0)) = trap.downcast_ref::() { - log::trace!( - "exited thread id = {} via `wasi::proc_exit` call", - wasi_thread_id - ) - } else { - panic!("{}", fail(trap.to_string())) - } + Err(e) => { + log::trace!("exiting thread id = {} due to error", wasi_thread_id); + let e = maybe_exit_on_error(e); + eprintln!("Error: {:?}", e); + std::process::exit(1); } } })?; diff --git a/crates/wasi/Cargo.toml b/crates/wasi/Cargo.toml index cca14f9597ce..73649f4afabd 100644 --- a/crates/wasi/Cargo.toml +++ b/crates/wasi/Cargo.toml @@ -13,6 +13,7 @@ include = ["src/**/*", "README.md", "LICENSE", "build.rs"] build = "build.rs" [dependencies] +libc = "0.2.60" wasi-common = { workspace = true } wasi-cap-std-sync = { workspace = true, optional = true } wasi-tokio = { workspace = true, optional = true } diff --git a/crates/wasi/src/lib.rs b/crates/wasi/src/lib.rs index a86557769515..6661751017e0 100644 --- a/crates/wasi/src/lib.rs +++ b/crates/wasi/src/lib.rs @@ -82,3 +82,45 @@ pub mod snapshots { } } } + +/// Exit the process with a conventional OS error code as long as Wasmtime +/// understands the error. If the error is not an `I32Exit` or `Trap`, return +/// the error back to the caller for it to decide what to do. +/// +/// Note: this function is designed for CLI usage of Wasmtime; embedders of +/// Wasmtime may want different error handling. +pub fn maybe_exit_on_error(e: anyhow::Error) -> anyhow::Error { + use std::process; + use wasmtime::Trap; + + // If a specific WASI error code was requested then that's + // forwarded through to the process here without printing any + // extra error information. + if let Some(exit) = e.downcast_ref::() { + // Print the error message in the usual way. + // On Windows, exit status 3 indicates an abort (see below), + // so return 1 indicating a non-zero status to avoid ambiguity. + if cfg!(windows) && exit.0 >= 3 { + process::exit(1); + } + process::exit(exit.0); + } + + // If the program exited because of a trap, return an error code + // to the outside environment indicating a more severe problem + // than a simple failure. + if e.is::() { + eprintln!("Error: {:?}", e); + + if cfg!(unix) { + // On Unix, return the error code of an abort. + process::exit(128 + libc::SIGABRT); + } else if cfg!(windows) { + // On Windows, return 3. + // https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/abort?view=vs-2019 + process::exit(3); + } + } + + e +} diff --git a/src/commands/run.rs b/src/commands/run.rs index 5800a8b436d2..4be6a3e8acfc 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -3,18 +3,15 @@ use anyhow::{anyhow, bail, Context as _, Result}; use clap::Parser; use once_cell::sync::Lazy; +use std::ffi::OsStr; +use std::path::{Component, Path, PathBuf}; use std::sync::Arc; use std::thread; use std::time::Duration; -use std::{ - ffi::OsStr, - path::{Component, Path, PathBuf}, - process, -}; -use wasmtime::{Engine, Func, Linker, Module, Store, Trap, Val, ValType}; +use wasmtime::{Engine, Func, Linker, Module, Store, Val, ValType}; use wasmtime_cli_flags::{CommonOptions, WasiModules}; +use wasmtime_wasi::maybe_exit_on_error; use wasmtime_wasi::sync::{ambient_authority, Dir, TcpListener, WasiCtxBuilder}; -use wasmtime_wasi::I32Exit; #[cfg(feature = "wasi-nn")] use wasmtime_wasi_nn::WasiNnCtx; @@ -222,38 +219,10 @@ impl RunCommand { { Ok(()) => (), Err(e) => { - // If a specific WASI error code was requested then that's - // forwarded through to the process here without printing any - // extra error information. - if let Some(exit) = e.downcast_ref::() { - // Print the error message in the usual way. - // On Windows, exit status 3 indicates an abort (see below), - // so return 1 indicating a non-zero status to avoid ambiguity. - if cfg!(windows) && exit.0 >= 3 { - process::exit(1); - } - process::exit(exit.0); - } - - // If the program exited because of a trap, return an error code - // to the outside environment indicating a more severe problem - // than a simple failure. - if e.is::() { - eprintln!("Error: {:?}", e); - - if cfg!(unix) { - // On Unix, return the error code of an abort. - process::exit(128 + libc::SIGABRT); - } else if cfg!(windows) { - // On Windows, return 3. - // https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/abort?view=vs-2019 - process::exit(3); - } - } - - // Otherwise fall back on Rust's default error printing/return + // Exit the process if Wasmtime understands the error; + // otherwise, fall back on Rust's default error printing/return // code. - return Err(e); + return Err(maybe_exit_on_error(e)); } } From 82ce2dec722f05b21a414f1f0feaa3afd2f6b1e4 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 26 Jan 2023 10:08:09 -0800 Subject: [PATCH 05/33] review: work around `fd_fdstat_set_flags` In order to make progress with wasi-threads, this change temporarily works around limitations induced by `wasi-common`'s `fd_fdstat_set_flags` to allow `&mut self` use in the implementation. Eventual resolution is tracked in https://github.com/bytecodealliance/wasmtime/issues/5643. This change makes several related helper functions (e.g., `set_fdflags`) take `&mut self` as well. Co-authored-by: Alex Crichton --- Cargo.lock | 1 + crates/wasi-common/Cargo.toml | 1 + crates/wasi-common/cap-std-sync/src/file.rs | 4 +- crates/wasi-common/cap-std-sync/src/net.rs | 6 +- crates/wasi-common/src/ctx.rs | 46 ++++++++++--- crates/wasi-common/src/file.rs | 11 +++- crates/wasi-common/src/snapshots/preview_1.rs | 19 ++++-- crates/wasi-common/src/table.rs | 16 +++++ crates/wasi-common/tokio/src/file.rs | 2 +- src/commands/run.rs | 66 +++++++------------ 10 files changed, 106 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f2e2b6d4d43f..022c0278e099 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3149,6 +3149,7 @@ dependencies = [ "cap-rand", "cap-std", "io-extras", + "log", "rustix", "thiserror", "tracing", diff --git a/crates/wasi-common/Cargo.toml b/crates/wasi-common/Cargo.toml index 065a03b52638..3e77c5c8488f 100644 --- a/crates/wasi-common/Cargo.toml +++ b/crates/wasi-common/Cargo.toml @@ -26,6 +26,7 @@ tracing = { workspace = true } cap-std = { workspace = true } cap-rand = { workspace = true } bitflags = { workspace = true } +log = { workspace = true } [target.'cfg(unix)'.dependencies] rustix = { workspace = true, features = ["fs"] } diff --git a/crates/wasi-common/cap-std-sync/src/file.rs b/crates/wasi-common/cap-std-sync/src/file.rs index c54a5ead7145..c184486a7f24 100644 --- a/crates/wasi-common/cap-std-sync/src/file.rs +++ b/crates/wasi-common/cap-std-sync/src/file.rs @@ -93,7 +93,7 @@ impl WasiFile for File { let fdflags = get_fd_flags(&*file)?; Ok(fdflags) } - async fn set_fdflags(&self, fdflags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { if fdflags.intersects( wasi_common::file::FdFlags::DSYNC | wasi_common::file::FdFlags::SYNC @@ -101,7 +101,7 @@ impl WasiFile for File { ) { return Err(Error::invalid_argument().context("cannot set DSYNC, SYNC, or RSYNC flag")); } - let mut file = self.0.write().unwrap(); + let file = self.0.get_mut().unwrap(); let set_fd_flags = (*file).new_set_fd_flags(to_sysif_fdflags(fdflags))?; (*file).set_fd_flags(set_fd_flags)?; Ok(()) diff --git a/crates/wasi-common/cap-std-sync/src/net.rs b/crates/wasi-common/cap-std-sync/src/net.rs index b46d8d7dafaf..921622d68d96 100644 --- a/crates/wasi-common/cap-std-sync/src/net.rs +++ b/crates/wasi-common/cap-std-sync/src/net.rs @@ -98,7 +98,7 @@ macro_rules! wasi_listen_write_impl { } async fn sock_accept(&self, fdflags: FdFlags) -> Result, Error> { let (stream, _) = self.0.accept()?; - let stream = <$stream>::from_cap_std(stream); + let mut stream = <$stream>::from_cap_std(stream); stream.set_fdflags(fdflags).await?; Ok(Box::new(stream)) } @@ -110,7 +110,7 @@ macro_rules! wasi_listen_write_impl { let fdflags = get_fd_flags(&self.0)?; Ok(fdflags) } - async fn set_fdflags(&self, fdflags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { if fdflags == wasi_common::file::FdFlags::NONBLOCK { self.0.set_nonblocking(true)?; } else if fdflags.is_empty() { @@ -197,7 +197,7 @@ macro_rules! wasi_stream_write_impl { let fdflags = get_fd_flags(&self.0)?; Ok(fdflags) } - async fn set_fdflags(&self, fdflags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { if fdflags == wasi_common::file::FdFlags::NONBLOCK { self.0.set_nonblocking(true)?; } else if fdflags.is_empty() { diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 1a115e0d8d43..f6b08e1cfef6 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -2,17 +2,26 @@ use crate::clocks::WasiClocks; use crate::dir::{DirCaps, DirEntry, WasiDir}; use crate::file::{FileCaps, FileEntry, WasiFile}; use crate::sched::WasiSched; -use crate::string_array::{StringArray, StringArrayError}; +use crate::string_array::StringArray; use crate::table::Table; -use crate::Error; +use crate::{Error, StringArrayError}; use cap_rand::RngCore; +use std::ops::Deref; use std::path::{Path, PathBuf}; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; -pub struct WasiCtx { +/// An `Arc`-wrapper around the wasi-common context to allow mutable access to +/// the file descriptor table. This wrapper is only necessary due to the +/// signature of `fd_fdstat_set_flags`; if that changes, there are a variety of +/// improvements that can be made (TODO: +/// https://github.com/bytecodealliance/wasmtime/issues/5643). +#[derive(Clone)] +pub struct WasiCtx(Arc); + +pub struct WasiCtxInner { pub args: StringArray, pub env: StringArray, - pub random: Box, + pub random: Mutex>, pub clocks: WasiClocks, pub sched: Box, pub table: Table, @@ -25,14 +34,14 @@ impl WasiCtx { sched: Box, table: Table, ) -> Self { - let s = WasiCtx { + let s = WasiCtx(Arc::new(WasiCtxInner { args: StringArray::new(), env: StringArray::new(), - random, + random: Mutex::new(random), clocks, sched, table, - }; + })); s.set_stdin(Box::new(crate::pipe::ReadPipe::new(std::io::empty()))); s.set_stdout(Box::new(crate::pipe::WritePipe::new(std::io::sink()))); s.set_stderr(Box::new(crate::pipe::WritePipe::new(std::io::sink()))); @@ -77,12 +86,22 @@ impl WasiCtx { &self.table } + pub fn table_mut(&mut self) -> Option<&mut Table> { + Arc::get_mut(&mut self.0).map(|c| &mut c.table) + } + pub fn push_arg(&mut self, arg: &str) -> Result<(), StringArrayError> { - self.args.push(arg.to_owned()) + let s = Arc::get_mut(&mut self.0).expect( + "`push_arg` should only be used during initialization before the context is cloned", + ); + s.args.push(arg.to_owned()) } pub fn push_env(&mut self, var: &str, value: &str) -> Result<(), StringArrayError> { - self.env.push(format!("{}={}", var, value))?; + let s = Arc::get_mut(&mut self.0).expect( + "`push_env` should only be used during initialization before the context is cloned", + ); + s.env.push(format!("{}={}", var, value))?; Ok(()) } @@ -130,3 +149,10 @@ impl WasiCtx { Ok(()) } } + +impl Deref for WasiCtx { + type Target = WasiCtxInner; + fn deref(&self) -> &Self::Target { + &self.0 + } +} diff --git a/crates/wasi-common/src/file.rs b/crates/wasi-common/src/file.rs index b76278373a65..828307255e56 100644 --- a/crates/wasi-common/src/file.rs +++ b/crates/wasi-common/src/file.rs @@ -64,7 +64,7 @@ pub trait WasiFile: Send + Sync { Ok(FdFlags::empty()) } - async fn set_fdflags(&self, _flags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&mut self, _flags: FdFlags) -> Result<(), Error> { Err(Error::badf()) } @@ -217,11 +217,15 @@ pub struct Filestat { pub(crate) trait TableFileExt { fn get_file(&self, fd: u32) -> Result, Error>; + fn get_file_mut(&mut self, fd: u32) -> Result<&mut FileEntry, Error>; } impl TableFileExt for crate::table::Table { fn get_file(&self, fd: u32) -> Result, Error> { self.get(fd) } + fn get_file_mut(&mut self, fd: u32) -> Result<&mut FileEntry, Error> { + self.get_mut(fd) + } } pub(crate) struct FileEntry { @@ -272,6 +276,7 @@ impl FileEntry { pub trait FileEntryExt { fn get_cap(&self, caps: FileCaps) -> Result<&dyn WasiFile, Error>; + fn get_cap_mut(&mut self, caps: FileCaps) -> Result<&mut dyn WasiFile, Error>; } impl FileEntryExt for FileEntry { @@ -279,6 +284,10 @@ impl FileEntryExt for FileEntry { self.capable_of(caps)?; Ok(&*self.file) } + fn get_cap_mut(&mut self, caps: FileCaps) -> Result<&mut dyn WasiFile, Error> { + self.capable_of(caps)?; + Ok(&mut *self.file) + } } bitflags! { diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 094270d51d3b..76ead4f090f7 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -189,11 +189,16 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { fd: types::Fd, flags: types::Fdflags, ) -> Result<(), Error> { - self.table() - .get_file(u32::from(fd))? - .get_cap(FileCaps::FDSTAT_SET_FLAGS)? - .set_fdflags(FdFlags::from(flags)) - .await + if let Some(table) = self.table_mut() { + table + .get_file_mut(u32::from(fd))? + .get_cap_mut(FileCaps::FDSTAT_SET_FLAGS)? + .set_fdflags(FdFlags::from(flags)) + .await + } else { + log::warn!("`fd_fdstat_set_flags` does not work with wasi-threads enabled; see https://github.com/bytecodealliance/wasmtime/issues/5643"); + Err(Error::invalid_argument()) + } } async fn fd_fdstat_set_rights( @@ -1110,7 +1115,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { while copied < buf.len() { let len = (buf.len() - copied).min(MAX_SHARED_BUFFER_SIZE as u32); let mut tmp = vec![0; len as usize]; - self.random.try_fill_bytes(&mut tmp)?; + self.random.lock().unwrap().try_fill_bytes(&mut tmp)?; let dest = buf .get_range(copied..copied + len) .unwrap() @@ -1122,7 +1127,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { // If the Wasm memory is non-shared, copy directly into the linear // memory. let mem = &mut buf.as_slice_mut()?.unwrap(); - self.random.try_fill_bytes(mem)?; + self.random.lock().unwrap().try_fill_bytes(mem)?; } Ok(()) } diff --git a/crates/wasi-common/src/table.rs b/crates/wasi-common/src/table.rs index a92fefff6a2a..40069636786e 100644 --- a/crates/wasi-common/src/table.rs +++ b/crates/wasi-common/src/table.rs @@ -76,6 +76,22 @@ impl Table { } } + /// Get a mutable reference to a resource of a given type at a given index. + /// Only one such reference can be borrowed at any given time. + pub fn get_mut(&mut self, key: u32) -> Result<&mut T, Error> { + let entry = match self.0.get_mut().unwrap().map.get_mut(&key) { + Some(entry) => entry, + None => return Err(Error::badf().context("key not in table")), + }; + let entry = match Arc::get_mut(entry) { + Some(entry) => entry, + None => return Err(Error::badf().context("cannot mutably borrow shared file")), + }; + entry + .downcast_mut::() + .ok_or_else(|| Error::badf().context("element is a different type")) + } + /// Remove a resource at a given index from the table. Returns the resource /// if it was present. pub fn delete(&self, key: u32) -> Option> { diff --git a/crates/wasi-common/tokio/src/file.rs b/crates/wasi-common/tokio/src/file.rs index 91e70aa58397..0217e315b783 100644 --- a/crates/wasi-common/tokio/src/file.rs +++ b/crates/wasi-common/tokio/src/file.rs @@ -116,7 +116,7 @@ macro_rules! wasi_file_impl { async fn get_fdflags(&self) -> Result { block_on_dummy_executor(|| self.0.get_fdflags()) } - async fn set_fdflags(&self, fdflags: FdFlags) -> Result<(), Error> { + async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { block_on_dummy_executor(|| self.0.set_fdflags(fdflags)) } async fn get_filestat(&self) -> Result { diff --git a/src/commands/run.rs b/src/commands/run.rs index 4be6a3e8acfc..fa6b6cc6ed8f 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -178,12 +178,11 @@ impl RunCommand { // Read the wasm module binary either as `*.wat` or a raw binary. let module = self.load_module(linker.engine(), &self.module)?; - let mut host = Arc::new(Host::default()); - let mut store = Store::new(&engine, host.clone()); + let mut host = Host::default(); + let mut store = Store::new(&engine, host); populate_with_wasi( - &mut host, &mut linker, - &store, + &mut store, module.clone(), preopen_dirs, &argv, @@ -290,8 +289,8 @@ impl RunCommand { fn load_main_module( &self, - store: &mut Store>, - linker: &mut Linker>, + store: &mut Store, + linker: &mut Linker, module: Module, ) -> Result<()> { if let Some(timeout) = self.wasm_timeout { @@ -324,8 +323,8 @@ impl RunCommand { fn invoke_export( &self, - store: &mut Store>, - linker: &Linker>, + store: &mut Store, + linker: &Linker, name: &str, ) -> Result<()> { let func = match linker @@ -339,12 +338,7 @@ impl RunCommand { self.invoke_func(store, func, Some(name)) } - fn invoke_func( - &self, - store: &mut Store>, - func: Func, - name: Option<&str>, - ) -> Result<()> { + fn invoke_func(&self, store: &mut Store, func: Func, name: Option<&str>) -> Result<()> { let ty = func.ty(&store); if ty.params().len() > 0 { eprintln!( @@ -419,23 +413,22 @@ impl RunCommand { } } -#[derive(Default)] +#[derive(Default, Clone)] struct Host { wasi: Option, #[cfg(feature = "wasi-crypto")] - wasi_crypto: Option, + wasi_crypto: Option>, #[cfg(feature = "wasi-nn")] - wasi_nn: Option, + wasi_nn: Option>, #[cfg(feature = "wasi-threads")] - wasi_threads: Option>>, + wasi_threads: Option>>, } /// Populates the given `Linker` with WASI APIs. fn populate_with_wasi( - host: &mut Arc, - linker: &mut Linker>, - store: &Store>, - module: Module, + linker: &mut Linker, + store: &mut Store, + _module: Module, preopen_dirs: Vec<(String, Dir)>, argv: &[String], vars: &[(String, String)], @@ -444,9 +437,7 @@ fn populate_with_wasi( mut tcplisten: Vec, ) -> Result<()> { if wasi_modules.wasi_common { - wasmtime_wasi::add_to_linker(linker, |host| { - Arc::get_mut(host).unwrap().wasi.as_mut().unwrap() - })?; + wasmtime_wasi::add_to_linker(linker, |host| host.wasi.as_mut().unwrap())?; let mut builder = WasiCtxBuilder::new(); builder = builder.inherit_stdio().args(argv)?.envs(vars)?; @@ -468,9 +459,7 @@ fn populate_with_wasi( builder = builder.preopened_dir(dir, name)?; } - Arc::get_mut(host) - .expect("there must be no other host references during setup") - .wasi = Some(builder.build()); + store.data_mut().wasi = Some(builder.build()); } if wasi_modules.wasi_crypto { @@ -481,14 +470,11 @@ fn populate_with_wasi( #[cfg(feature = "wasi-crypto")] { wasmtime_wasi_crypto::add_to_linker(linker, |host| { - Arc::get_mut(host) - .wasi_crypto + host.wasi_crypto .as_mut() .expect("wasi-crypto is not implemented with multi-threading support") })?; - Arc::get_mut(host) - .expect("there must be no other host references during setup") - .wasi_crypto = Some(WasiCryptoCtx::new()); + store.data_mut().wasi_crypto = Some(Arc::new(WasiCryptoCtx::new())); } } @@ -500,14 +486,11 @@ fn populate_with_wasi( #[cfg(feature = "wasi-nn")] { wasmtime_wasi_nn::add_to_linker(linker, |host| { - Arc::get_mut(host) - .wasi_nn + host.wasi_nn .as_mut() .expect("wasi-nn is not implemented with multi-threading support") })?; - Arc::get_mut(host) - .expect("there must be no other host references during setup") - .wasi_nn = Some(WasiNnCtx::new()?); + store.data_mut().wasi_nn = Some(Arc::new(WasiNnCtx::new()?)); } } @@ -518,12 +501,11 @@ fn populate_with_wasi( } #[cfg(feature = "wasi-threads")] { - wasmtime_wasi_threads::add_to_linker(linker, store, &module, |host| { + wasmtime_wasi_threads::add_to_linker(linker, store, &_module, |host| { host.wasi_threads.as_ref().unwrap() })?; - Arc::get_mut(host) - .expect("there must be no other host references during setup") - .wasi_threads = Some(WasiThreadsCtx::new(module, linker.clone())); + store.data_mut().wasi_threads = + Some(Arc::new(WasiThreadsCtx::new(_module, linker.clone()))); } } From 9fa3bdf929f22ae8c018b3142e53754c58868dee Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 26 Jan 2023 18:00:10 -0800 Subject: [PATCH 06/33] fix: avoid unused import --- src/commands/run.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/commands/run.rs b/src/commands/run.rs index fa6b6cc6ed8f..9e3a5f873fba 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -5,7 +5,6 @@ use clap::Parser; use once_cell::sync::Lazy; use std::ffi::OsStr; use std::path::{Component, Path, PathBuf}; -use std::sync::Arc; use std::thread; use std::time::Duration; use wasmtime::{Engine, Func, Linker, Module, Store, Val, ValType}; @@ -417,11 +416,11 @@ impl RunCommand { struct Host { wasi: Option, #[cfg(feature = "wasi-crypto")] - wasi_crypto: Option>, + wasi_crypto: Option>, #[cfg(feature = "wasi-nn")] - wasi_nn: Option>, + wasi_nn: Option>, #[cfg(feature = "wasi-threads")] - wasi_threads: Option>>, + wasi_threads: Option>>, } /// Populates the given `Linker` with WASI APIs. @@ -474,7 +473,7 @@ fn populate_with_wasi( .as_mut() .expect("wasi-crypto is not implemented with multi-threading support") })?; - store.data_mut().wasi_crypto = Some(Arc::new(WasiCryptoCtx::new())); + store.data_mut().wasi_crypto = Some(std::sync::Arc::new(WasiCryptoCtx::new())); } } @@ -490,7 +489,7 @@ fn populate_with_wasi( .as_mut() .expect("wasi-nn is not implemented with multi-threading support") })?; - store.data_mut().wasi_nn = Some(Arc::new(WasiNnCtx::new()?)); + store.data_mut().wasi_nn = Some(std::sync::Arc::new(WasiNnCtx::new()?)); } } @@ -504,8 +503,10 @@ fn populate_with_wasi( wasmtime_wasi_threads::add_to_linker(linker, store, &_module, |host| { host.wasi_threads.as_ref().unwrap() })?; - store.data_mut().wasi_threads = - Some(Arc::new(WasiThreadsCtx::new(_module, linker.clone()))); + store.data_mut().wasi_threads = Some(std::sync::Arc::new(WasiThreadsCtx::new( + _module, + linker.clone(), + ))); } } From 096a882bd8e5449b26e1d3e5e225ed56bd0519e9 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 26 Jan 2023 18:03:07 -0800 Subject: [PATCH 07/33] fix: add wasmtime-wasi-threads to publish script --- scripts/publish.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/publish.rs b/scripts/publish.rs index fc2c6c8bf499..df377254c2e8 100644 --- a/scripts/publish.rs +++ b/scripts/publish.rs @@ -64,8 +64,9 @@ const CRATES_TO_PUBLISH: &[&str] = &[ "wasi-tokio", // other misc wasmtime crates "wasmtime-wasi", - "wasmtime-wasi-nn", "wasmtime-wasi-crypto", + "wasmtime-wasi-nn", + "wasmtime-wasi-threads", "wasmtime-wast", "wasmtime-cli-flags", "wasmtime-cli", @@ -84,8 +85,9 @@ const PUBLIC_CRATES: &[&str] = &[ // patch releases. "wasmtime", "wasmtime-wasi", - "wasmtime-wasi-nn", "wasmtime-wasi-crypto", + "wasmtime-wasi-nn", + "wasmtime-wasi-threads", "wasmtime-cli", // all cranelift crates are considered "public" in that they can't // have breaking API changes in patch releases From ac85c6649ba3f610bcec9ee4a74c75f53536710d Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 26 Jan 2023 19:43:04 -0800 Subject: [PATCH 08/33] fix: use `Arc::get_mut` for other WASI proposals --- src/commands/run.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/commands/run.rs b/src/commands/run.rs index 9e3a5f873fba..932a35c4b9de 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -469,8 +469,7 @@ fn populate_with_wasi( #[cfg(feature = "wasi-crypto")] { wasmtime_wasi_crypto::add_to_linker(linker, |host| { - host.wasi_crypto - .as_mut() + std::sync::Arc::get_mut(host.wasi_crypto.as_mut().unwrap()) .expect("wasi-crypto is not implemented with multi-threading support") })?; store.data_mut().wasi_crypto = Some(std::sync::Arc::new(WasiCryptoCtx::new())); @@ -485,8 +484,7 @@ fn populate_with_wasi( #[cfg(feature = "wasi-nn")] { wasmtime_wasi_nn::add_to_linker(linker, |host| { - host.wasi_nn - .as_mut() + std::sync::Arc::get_mut(host.wasi_nn.as_mut().unwrap()) .expect("wasi-nn is not implemented with multi-threading support") })?; store.data_mut().wasi_nn = Some(std::sync::Arc::new(WasiNnCtx::new()?)); From 950420d6ac17b742d26b5d394bc22c0a96181935 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 26 Jan 2023 19:58:24 -0800 Subject: [PATCH 09/33] fix: re-add default wasi-nn feature --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 42d76b246130..74fee9c1eba7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -201,6 +201,7 @@ default = [ "wasmtime/wat", "wasmtime/parallel-compilation", "vtune", + "wasi-nn", "pooling-allocator", ] jitdump = ["wasmtime/jitdump"] From b3066edd47d36651fdb5fe9ce74827dbb4c007b4 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 30 Jan 2023 15:18:17 -0800 Subject: [PATCH 10/33] review: fix redundant workspace member --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 74fee9c1eba7..c826666461cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,6 @@ members = [ "crates/bench-api", "crates/c-api", "crates/cli-flags", - "crates/wasi-threads", "crates/environ/fuzz", "crates/jit-icache-coherence", "crates/winch", From 20fa82e1e87b2cda583ad8d9db01c158a4faabaf Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 30 Jan 2023 15:25:20 -0800 Subject: [PATCH 11/33] review: add TODO to remove randomness mutex --- crates/wasi-common/src/ctx.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index f6b08e1cfef6..98ca0f79d705 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -19,8 +19,11 @@ use std::sync::{Arc, Mutex}; pub struct WasiCtx(Arc); pub struct WasiCtxInner { - pub args: StringArray, - pub env: StringArray, +pub args: StringArray, +pub env: StringArray, + // TODO: this mutex should not be necessary, it forces threads to serialize + // their access to randomness unnecessarily + // (https://github.com/bytecodealliance/wasmtime/issues/5660). pub random: Mutex>, pub clocks: WasiClocks, pub sched: Box, From ec2ff2cd8e93d4080b0efab3fdea9e2fc1795066 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 30 Jan 2023 17:32:47 -0800 Subject: [PATCH 12/33] review: improve thread exit documentation --- crates/wasi-threads/src/lib.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/wasi-threads/src/lib.rs b/crates/wasi-threads/src/lib.rs index ebf0ac51788c..40f7c461fe8c 100644 --- a/crates/wasi-threads/src/lib.rs +++ b/crates/wasi-threads/src/lib.rs @@ -51,10 +51,11 @@ impl WasiThreadsCtx { WASI_ENTRY_POINT ))); - // Start the thread's entry point; any failures are simply printed - // before exiting the thread. It may be necessary to handle failures - // here somehow, e.g., so that `pthread_join` can be notified if the - // user function traps for some reason (TODO). + // Start the thread's entry point. Any traps or calls to + // `proc_exit`, by specification, should end execution for all + // threads. This code uses `process::exit` to do so, which is what + // the user expects from the CLI but probably not in a Wasmtime + // embedding. log::trace!( "spawned thread id = {}; calling start function `{}` with: {}", wasi_thread_id, @@ -62,7 +63,7 @@ impl WasiThreadsCtx { thread_start_arg ); match thread_entry_point.call(&mut store, (wasi_thread_id, thread_start_arg)) { - Ok(_) => {} + Ok(_) => log::trace!("exiting thread id = {} normally", wasi_thread_id), Err(e) => { log::trace!("exiting thread id = {} due to error", wasi_thread_id); let e = maybe_exit_on_error(e); From 0078f819cf2f27e27506620eed2da24775b3c11f Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 12:02:54 -0800 Subject: [PATCH 13/33] review: wrap `Linker` with an `Arc` for faster cloning Co-authored-by: Alex Crichton --- crates/wasi-threads/src/lib.rs | 6 +++--- src/commands/run.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/wasi-threads/src/lib.rs b/crates/wasi-threads/src/lib.rs index 40f7c461fe8c..3ec46e3a0b98 100644 --- a/crates/wasi-threads/src/lib.rs +++ b/crates/wasi-threads/src/lib.rs @@ -4,7 +4,7 @@ use anyhow::{anyhow, Result}; use rand::Rng; -use std::thread; +use std::sync::Arc; use wasmtime::{Caller, Linker, Module, SharedMemory, Store}; use wasmtime_wasi::maybe_exit_on_error; @@ -14,11 +14,11 @@ const WASI_ENTRY_POINT: &str = "wasi_thread_start"; pub struct WasiThreadsCtx { module: Module, - linker: Linker, + linker: Arc>, } impl WasiThreadsCtx { - pub fn new(module: Module, linker: Linker) -> Self { + pub fn new(module: Module, linker: Arc>) -> Self { Self { module, linker } } diff --git a/src/commands/run.rs b/src/commands/run.rs index 932a35c4b9de..4c02f0325daf 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -503,7 +503,7 @@ fn populate_with_wasi( })?; store.data_mut().wasi_threads = Some(std::sync::Arc::new(WasiThreadsCtx::new( _module, - linker.clone(), + std::sync::Arc::new(linker.clone()), ))); } } From 39235269768cbb5f02f713a7d31c29d56f91f56b Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 12:05:19 -0800 Subject: [PATCH 14/33] fix: allow wasi-nn to compile with threads changes Co-authored-by: Alex Crichton --- crates/wasi-nn/src/api.rs | 4 ++-- crates/wasi-nn/src/openvino.rs | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/wasi-nn/src/api.rs b/crates/wasi-nn/src/api.rs index 89fd46fbdc2f..2ad6e0edf94e 100644 --- a/crates/wasi-nn/src/api.rs +++ b/crates/wasi-nn/src/api.rs @@ -7,7 +7,7 @@ use thiserror::Error; use wiggle::GuestError; /// A [Backend] contains the necessary state to load [BackendGraph]s. -pub(crate) trait Backend: Send { +pub(crate) trait Backend: Send + Sync { fn name(&self) -> &str; fn load( &mut self, @@ -18,7 +18,7 @@ pub(crate) trait Backend: Send { /// A [BackendGraph] can create [BackendExecutionContext]s; this is the backing /// implementation for a [crate::witx::types::Graph]. -pub(crate) trait BackendGraph: Send { +pub(crate) trait BackendGraph: Send + Sync { fn init_execution_context(&mut self) -> Result, BackendError>; } diff --git a/crates/wasi-nn/src/openvino.rs b/crates/wasi-nn/src/openvino.rs index fff9bf7e5cf1..769beb3dad70 100644 --- a/crates/wasi-nn/src/openvino.rs +++ b/crates/wasi-nn/src/openvino.rs @@ -1,4 +1,5 @@ //! Implements the wasi-nn API. + use crate::api::{Backend, BackendError, BackendExecutionContext, BackendGraph}; use crate::witx::types::{ExecutionTarget, GraphBuilderArray, Tensor, TensorType}; use openvino::{InferenceError, Layout, Precision, SetupError, TensorDesc}; @@ -7,6 +8,9 @@ use std::sync::Arc; #[derive(Default)] pub(crate) struct OpenvinoBackend(Option); +unsafe impl Send for OpenvinoBackend {} +unsafe impl Sync for OpenvinoBackend {} + impl Backend for OpenvinoBackend { fn name(&self) -> &str { "openvino" @@ -65,6 +69,9 @@ impl Backend for OpenvinoBackend { struct OpenvinoGraph(Arc, openvino::ExecutableNetwork); +unsafe impl Send for OpenvinoGraph {} +unsafe impl Sync for OpenvinoGraph {} + impl BackendGraph for OpenvinoGraph { fn init_execution_context(&mut self) -> Result, BackendError> { let infer_request = self.1.create_infer_request()?; From 2959abc16dbbb9242d682243ae36fb35056dcb52 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 12:06:40 -0800 Subject: [PATCH 15/33] review: back out no-longer-needed changes from #5326 Co-authored-by: Alex Crichton --- crates/wasi-common/cap-std-sync/src/file.rs | 125 ++++++++---------- crates/wasi-common/cap-std-sync/src/net.rs | 22 ++- .../cap-std-sync/src/sched/unix.rs | 11 +- crates/wasi-common/cap-std-sync/src/stdio.rs | 22 ++- crates/wasi-common/src/file.rs | 4 +- 5 files changed, 74 insertions(+), 110 deletions(-) diff --git a/crates/wasi-common/cap-std-sync/src/file.rs b/crates/wasi-common/cap-std-sync/src/file.rs index c184486a7f24..49a86b8298d2 100644 --- a/crates/wasi-common/cap-std-sync/src/file.rs +++ b/crates/wasi-common/cap-std-sync/src/file.rs @@ -5,7 +5,6 @@ use is_terminal::IsTerminal; use std::any::Any; use std::convert::TryInto; use std::io; -use std::sync::{Arc, RwLock, RwLockReadGuard}; use system_interface::{ fs::{FileIoExt, GetSetFdFlags}, io::{IoExt, ReadReady}, @@ -15,48 +14,11 @@ use wasi_common::{ Error, ErrorExt, }; -#[cfg(unix)] -use io_lifetimes::{AsFd, BorrowedFd}; - -#[cfg(windows)] -use io_lifetimes::{AsHandle, BorrowedHandle}; - -#[cfg(windows)] -use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; - -pub struct BorrowedFile<'a>(RwLockReadGuard<'a, cap_std::fs::File>); - -#[cfg(unix)] -impl AsFd for BorrowedFile<'_> { - fn as_fd(&self) -> BorrowedFd<'_> { - self.0.as_fd() - } -} - -#[cfg(windows)] -impl AsHandle for BorrowedFile<'_> { - fn as_handle(&self) -> BorrowedHandle<'_> { - self.0.as_handle() - } -} - -#[cfg(windows)] -impl AsRawHandleOrSocket for BorrowedFile<'_> { - #[inline] - fn as_raw_handle_or_socket(&self) -> RawHandleOrSocket { - self.0.as_raw_handle_or_socket() - } -} - -pub struct File(RwLock); +pub struct File(cap_std::fs::File); impl File { pub fn from_cap_std(file: cap_std::fs::File) -> Self { - File(RwLock::new(file)) - } - - pub fn borrow(&self) -> BorrowedFile { - BorrowedFile(self.0.read().unwrap()) + File(file) } } @@ -65,32 +27,28 @@ impl WasiFile for File { fn as_any(&self) -> &dyn Any { self } - #[cfg(unix)] - fn pollable(&self) -> Option> { - Some(Arc::new(self.borrow())) + fn pollable(&self) -> Option { + Some(self.0.as_fd()) } - #[cfg(windows)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedFile(self.0.read().unwrap()))) + fn pollable(&self) -> Option { + Some(self.0.as_raw_handle_or_socket()) } - async fn datasync(&self) -> Result<(), Error> { - self.0.read().unwrap().sync_data()?; + self.0.sync_data()?; Ok(()) } async fn sync(&self) -> Result<(), Error> { - self.0.read().unwrap().sync_all()?; + self.0.sync_all()?; Ok(()) } async fn get_filetype(&self) -> Result { - let meta = self.0.read().unwrap().metadata()?; + let meta = self.0.metadata()?; Ok(filetype_from(&meta.file_type())) } async fn get_fdflags(&self) -> Result { - let file = self.0.read().unwrap(); - let fdflags = get_fd_flags(&*file)?; + let fdflags = get_fd_flags(&self.0)?; Ok(fdflags) } async fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> { @@ -101,13 +59,12 @@ impl WasiFile for File { ) { return Err(Error::invalid_argument().context("cannot set DSYNC, SYNC, or RSYNC flag")); } - let file = self.0.get_mut().unwrap(); - let set_fd_flags = (*file).new_set_fd_flags(to_sysif_fdflags(fdflags))?; - (*file).set_fd_flags(set_fd_flags)?; + let set_fd_flags = self.0.new_set_fd_flags(to_sysif_fdflags(fdflags))?; + self.0.set_fd_flags(set_fd_flags)?; Ok(()) } async fn get_filestat(&self) -> Result { - let meta = self.0.read().unwrap().metadata()?; + let meta = self.0.metadata()?; Ok(Filestat { device_id: meta.dev(), inode: meta.ino(), @@ -120,18 +77,15 @@ impl WasiFile for File { }) } async fn set_filestat_size(&self, size: u64) -> Result<(), Error> { - self.0.read().unwrap().set_len(size)?; + self.0.set_len(size)?; Ok(()) } async fn advise(&self, offset: u64, len: u64, advice: Advice) -> Result<(), Error> { - self.0 - .read() - .unwrap() - .advise(offset, len, convert_advice(advice))?; + self.0.advise(offset, len, convert_advice(advice))?; Ok(()) } async fn allocate(&self, offset: u64, len: u64) -> Result<(), Error> { - self.0.read().unwrap().allocate(offset, len)?; + self.0.allocate(offset, len)?; Ok(()) } async fn set_times( @@ -140,13 +94,11 @@ impl WasiFile for File { mtime: Option, ) -> Result<(), Error> { self.0 - .read() - .unwrap() .set_times(convert_systimespec(atime), convert_systimespec(mtime))?; Ok(()) } async fn read_vectored<'a>(&self, bufs: &mut [io::IoSliceMut<'a>]) -> Result { - let n = self.0.read().unwrap().read_vectored(bufs)?; + let n = self.0.read_vectored(bufs)?; Ok(n.try_into()?) } async fn read_vectored_at<'a>( @@ -154,11 +106,11 @@ impl WasiFile for File { bufs: &mut [io::IoSliceMut<'a>], offset: u64, ) -> Result { - let n = self.0.read().unwrap().read_vectored_at(bufs, offset)?; + let n = self.0.read_vectored_at(bufs, offset)?; Ok(n.try_into()?) } async fn write_vectored<'a>(&self, bufs: &[io::IoSlice<'a>]) -> Result { - let n = self.0.read().unwrap().write_vectored(bufs)?; + let n = self.0.write_vectored(bufs)?; Ok(n.try_into()?) } async fn write_vectored_at<'a>( @@ -166,21 +118,21 @@ impl WasiFile for File { bufs: &[io::IoSlice<'a>], offset: u64, ) -> Result { - let n = self.0.read().unwrap().write_vectored_at(bufs, offset)?; + let n = self.0.write_vectored_at(bufs, offset)?; Ok(n.try_into()?) } async fn seek(&self, pos: std::io::SeekFrom) -> Result { - Ok(self.0.read().unwrap().seek(pos)?) + Ok(self.0.seek(pos)?) } async fn peek(&self, buf: &mut [u8]) -> Result { - let n = self.0.read().unwrap().peek(buf)?; + let n = self.0.peek(buf)?; Ok(n.try_into()?) } fn num_ready_bytes(&self) -> Result { - Ok(self.0.read().unwrap().num_ready_bytes()?) + Ok(self.0.num_ready_bytes()?) } fn isatty(&self) -> bool { - self.0.read().unwrap().is_terminal() + self.0.is_terminal() } } @@ -207,6 +159,35 @@ pub fn filetype_from(ft: &cap_std::fs::FileType) -> FileType { } } +#[cfg(windows)] +use io_lifetimes::{AsHandle, BorrowedHandle}; +#[cfg(windows)] +impl AsHandle for File { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.0.as_handle() + } +} + +#[cfg(windows)] +use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; +#[cfg(windows)] +impl AsRawHandleOrSocket for File { + #[inline] + fn as_raw_handle_or_socket(&self) -> RawHandleOrSocket { + self.0.as_raw_handle_or_socket() + } +} + +#[cfg(unix)] +use io_lifetimes::{AsFd, BorrowedFd}; + +#[cfg(unix)] +impl AsFd for File { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0.as_fd() + } +} + pub(crate) fn convert_systimespec( t: Option, ) -> Option { diff --git a/crates/wasi-common/cap-std-sync/src/net.rs b/crates/wasi-common/cap-std-sync/src/net.rs index 921622d68d96..c0750cd83e46 100644 --- a/crates/wasi-common/cap-std-sync/src/net.rs +++ b/crates/wasi-common/cap-std-sync/src/net.rs @@ -8,7 +8,6 @@ use io_lifetimes::{AsSocket, BorrowedSocket}; use std::any::Any; use std::convert::TryInto; use std::io; -use std::sync::Arc; #[cfg(unix)] use system_interface::fs::GetSetFdFlags; use system_interface::io::IoExt; @@ -19,11 +18,6 @@ use wasi_common::{ Error, ErrorExt, }; -#[cfg(unix)] -use wasi_common::file::BorrowedAsFd; -#[cfg(windows)] -use wasi_common::file::BorrowedAsRawHandleOrSocket; - pub enum Socket { TcpListener(cap_std::net::TcpListener), TcpStream(cap_std::net::TcpStream), @@ -89,12 +83,12 @@ macro_rules! wasi_listen_write_impl { self } #[cfg(unix)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedAsFd::new(&self.0))) + fn pollable(&self) -> Option { + Some(self.0.as_fd()) } #[cfg(windows)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedAsRawHandleOrSocket::new(&self.0))) + fn pollable(&self) -> Option { + Some(self.0.as_raw_handle_or_socket()) } async fn sock_accept(&self, fdflags: FdFlags) -> Result, Error> { let (stream, _) = self.0.accept()?; @@ -182,12 +176,12 @@ macro_rules! wasi_stream_write_impl { self } #[cfg(unix)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedAsFd::new(&self.0))) + fn pollable(&self) -> Option { + Some(self.0.as_fd()) } #[cfg(windows)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedAsRawHandleOrSocket::new(&self.0))) + fn pollable(&self) -> Option { + Some(self.0.as_raw_handle_or_socket()) } async fn get_filetype(&self) -> Result { Ok(FileType::SocketStream) diff --git a/crates/wasi-common/cap-std-sync/src/sched/unix.rs b/crates/wasi-common/cap-std-sync/src/sched/unix.rs index 000299e2721b..13f20f018812 100644 --- a/crates/wasi-common/cap-std-sync/src/sched/unix.rs +++ b/crates/wasi-common/cap-std-sync/src/sched/unix.rs @@ -8,7 +8,7 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> { if poll.is_empty() { return Ok(()); } - let mut poll_as_fds = Vec::new(); + let mut pollfds = Vec::new(); for s in poll.rw_subscriptions() { match s { Subscription::Read(f) => { @@ -16,7 +16,7 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> { .file .pollable() .ok_or(Error::invalid_argument().context("file is not pollable"))?; - poll_as_fds.push((fd, PollFlags::IN)); + pollfds.push(PollFd::from_borrowed_fd(fd, PollFlags::IN)); } Subscription::Write(f) => { @@ -24,17 +24,12 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> { .file .pollable() .ok_or(Error::invalid_argument().context("file is not pollable"))?; - poll_as_fds.push((fd, PollFlags::OUT)); + pollfds.push(PollFd::from_borrowed_fd(fd, PollFlags::OUT)); } Subscription::MonotonicClock { .. } => unreachable!(), } } - let mut pollfds = poll_as_fds - .iter() - .map(|(fd, events)| PollFd::from_borrowed_fd(fd.as_fd(), events.clone())) - .collect::>(); - let ready = loop { let poll_timeout = if let Some(t) = poll.earliest_clock_deadline() { let duration = t.duration_until().unwrap_or(Duration::from_secs(0)); diff --git a/crates/wasi-common/cap-std-sync/src/stdio.rs b/crates/wasi-common/cap-std-sync/src/stdio.rs index dd6f2344faf4..60f55056bccd 100644 --- a/crates/wasi-common/cap-std-sync/src/stdio.rs +++ b/crates/wasi-common/cap-std-sync/src/stdio.rs @@ -7,14 +7,8 @@ use std::convert::TryInto; use std::fs::File; use std::io; use std::io::{Read, Write}; -use std::sync::Arc; use system_interface::io::ReadReady; -#[cfg(unix)] -use wasi_common::file::BorrowedAsFd; -#[cfg(windows)] -use wasi_common::file::BorrowedAsRawHandleOrSocket; - #[cfg(windows)] use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; #[cfg(unix)] @@ -39,13 +33,13 @@ impl WasiFile for Stdin { } #[cfg(unix)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedAsFd::new(&self.0))) + fn pollable(&self) -> Option { + Some(self.0.as_fd()) } #[cfg(windows)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedAsRawHandleOrSocket::new(&self.0))) + fn pollable(&self) -> Option { + Some(self.0.as_raw_handle_or_socket()) } async fn get_filetype(&self) -> Result { @@ -116,12 +110,12 @@ macro_rules! wasi_file_write_impl { self } #[cfg(unix)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedAsFd::new(&self.0))) + fn pollable(&self) -> Option { + Some(self.0.as_fd()) } #[cfg(windows)] - fn pollable(&self) -> Option> { - Some(Arc::new(BorrowedAsRawHandleOrSocket::new(&self.0))) + fn pollable(&self) -> Option { + Some(self.0.as_raw_handle_or_socket()) } async fn get_filetype(&self) -> Result { if self.isatty() { diff --git a/crates/wasi-common/src/file.rs b/crates/wasi-common/src/file.rs index 828307255e56..5f4e8d8f80ac 100644 --- a/crates/wasi-common/src/file.rs +++ b/crates/wasi-common/src/file.rs @@ -15,12 +15,12 @@ pub trait WasiFile: Send + Sync { async fn get_filetype(&self) -> Result; #[cfg(unix)] - fn pollable(&self) -> Option> { + fn pollable(&self) -> Option { None } #[cfg(windows)] - fn pollable(&self) -> Option> { + fn pollable(&self) -> Option { None } From e1a29458e6f561196243de123131bd04eb459945 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 13:36:15 -0800 Subject: [PATCH 16/33] review: move entry point check earlier Co-authored-by: Alex Crichton --- crates/wasi-threads/src/lib.rs | 21 +++++++++++++++++++-- src/commands/run.rs | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/crates/wasi-threads/src/lib.rs b/crates/wasi-threads/src/lib.rs index 3ec46e3a0b98..14aaa38cfc70 100644 --- a/crates/wasi-threads/src/lib.rs +++ b/crates/wasi-threads/src/lib.rs @@ -18,8 +18,14 @@ pub struct WasiThreadsCtx { } impl WasiThreadsCtx { - pub fn new(module: Module, linker: Arc>) -> Self { - Self { module, linker } + pub fn new(module: Module, linker: Arc>) -> Result { + if !has_wasi_entry_point(&module) { + bail!( + "failed to find wasi-threads entry point function: {}", + WASI_ENTRY_POINT + ); + } + Ok(Self { module, linker }) } pub fn spawn(&self, host: T, thread_start_arg: i32) -> Result { @@ -134,3 +140,14 @@ pub fn add_to_linker( module should import a single shared memory as \"memory\"" )) } + +fn has_wasi_entry_point(module: &Module) -> bool { + module + .get_export(WASI_ENTRY_POINT) + .and_then(|t| t.func().cloned()) + .and_then(|t| { + let params: Vec = t.params().collect(); + Some(params == [ValType::I32, ValType::I32] && t.results().len() == 0) + }) + .unwrap_or(false) +} diff --git a/src/commands/run.rs b/src/commands/run.rs index 4c02f0325daf..e993385e97e7 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -504,7 +504,7 @@ fn populate_with_wasi( store.data_mut().wasi_threads = Some(std::sync::Arc::new(WasiThreadsCtx::new( _module, std::sync::Arc::new(linker.clone()), - ))); + )?)); } } From 8cfc619de6a38b06cbd3365dec00870e555cc236 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 13:38:50 -0800 Subject: [PATCH 17/33] review: catch host errors and exit all threads Co-authored-by: Alex Crichton --- crates/wasi-threads/src/lib.rs | 84 ++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/crates/wasi-threads/src/lib.rs b/crates/wasi-threads/src/lib.rs index 14aaa38cfc70..1c3fd8075e04 100644 --- a/crates/wasi-threads/src/lib.rs +++ b/crates/wasi-threads/src/lib.rs @@ -2,10 +2,12 @@ //! //! [`wasi-threads`]: https://github.com/WebAssembly/wasi-threads -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, bail, Result}; use rand::Rng; +use std::panic::{catch_unwind, AssertUnwindSafe}; use std::sync::Arc; -use wasmtime::{Caller, Linker, Module, SharedMemory, Store}; +use std::thread; +use wasmtime::{Caller, Linker, Module, SharedMemory, Store, ValType}; use wasmtime_wasi::maybe_exit_on_error; // This name is a function export designated by the wasi-threads specification: @@ -36,46 +38,50 @@ impl WasiThreadsCtx { let wasi_thread_id = random_thread_id(); let builder = thread::Builder::new().name(format!("wasi-thread-{}", wasi_thread_id)); builder.spawn(move || { - // Convenience function for printing failures, since the `Thread` - // has no way to report a failure to the outer context. - let fail = |msg: String| { - format!( - "wasi-thread-{} exited unsuccessfully: {}", - wasi_thread_id, msg - ) - }; + // Catch any panic failures in host code; e.g., if a WASI module + // were to crash, we want all threads to exit, not just this one. + let result = catch_unwind(AssertUnwindSafe(|| { + // Each new instance is created in its own store. + let mut store = Store::new(&module.engine(), host); - // Each new instance is created in its own store. - let mut store = Store::new(&module.engine(), host); - let instance = linker - .instantiate(&mut store, &module) - .expect(&fail("failed to instantiate".into())); - let thread_entry_point = instance - .get_typed_func::<(i32, i32), ()>(&mut store, WASI_ENTRY_POINT) - .expect(&fail(format!( - "failed to find wasi-threads entry point function: {}", - WASI_ENTRY_POINT - ))); + // Ideally, we would have already checked much earlier (e.g., + // `new`) whether the module can be instantiated. Because + // `Linker::instantiate_pre` requires a `Store` and that is only + // available now. TODO: + // https://github.com/bytecodealliance/wasmtime/issues/5675. + let instance = linker.instantiate(&mut store, &module).expect(&format!( + "wasi-thread-{} exited unsuccessfully: failed to instantiate", + wasi_thread_id + )); + let thread_entry_point = instance + .get_typed_func::<(i32, i32), ()>(&mut store, WASI_ENTRY_POINT) + .unwrap(); - // Start the thread's entry point. Any traps or calls to - // `proc_exit`, by specification, should end execution for all - // threads. This code uses `process::exit` to do so, which is what - // the user expects from the CLI but probably not in a Wasmtime - // embedding. - log::trace!( - "spawned thread id = {}; calling start function `{}` with: {}", - wasi_thread_id, - WASI_ENTRY_POINT, - thread_start_arg - ); - match thread_entry_point.call(&mut store, (wasi_thread_id, thread_start_arg)) { - Ok(_) => log::trace!("exiting thread id = {} normally", wasi_thread_id), - Err(e) => { - log::trace!("exiting thread id = {} due to error", wasi_thread_id); - let e = maybe_exit_on_error(e); - eprintln!("Error: {:?}", e); - std::process::exit(1); + // Start the thread's entry point. Any traps or calls to + // `proc_exit`, by specification, should end execution for all + // threads. This code uses `process::exit` to do so, which is what + // the user expects from the CLI but probably not in a Wasmtime + // embedding. + log::trace!( + "spawned thread id = {}; calling start function `{}` with: {}", + wasi_thread_id, + WASI_ENTRY_POINT, + thread_start_arg + ); + match thread_entry_point.call(&mut store, (wasi_thread_id, thread_start_arg)) { + Ok(_) => log::trace!("exiting thread id = {} normally", wasi_thread_id), + Err(e) => { + log::trace!("exiting thread id = {} due to error", wasi_thread_id); + let e = maybe_exit_on_error(e); + eprintln!("Error: {:?}", e); + std::process::exit(1); + } } + })); + + if let Err(e) = result { + eprintln!("Error: {:?}", e); + std::process::exit(1); } })?; From 9043f8430fe7410126572b756ac6347c7c73715d Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 13:41:33 -0800 Subject: [PATCH 18/33] fix: formatting --- crates/wasi-common/src/ctx.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 98ca0f79d705..6eabaa1fdee5 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -19,8 +19,8 @@ use std::sync::{Arc, Mutex}; pub struct WasiCtx(Arc); pub struct WasiCtxInner { -pub args: StringArray, -pub env: StringArray, + pub args: StringArray, + pub env: StringArray, // TODO: this mutex should not be necessary, it forces threads to serialize // their access to randomness unnecessarily // (https://github.com/bytecodealliance/wasmtime/issues/5660). From d58613f1e68cbad2baed803620415d90303b5d4b Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 17:07:52 -0800 Subject: [PATCH 19/33] review: change error text --- crates/wasi-threads/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasi-threads/src/lib.rs b/crates/wasi-threads/src/lib.rs index 1c3fd8075e04..9116e4508e3e 100644 --- a/crates/wasi-threads/src/lib.rs +++ b/crates/wasi-threads/src/lib.rs @@ -80,7 +80,7 @@ impl WasiThreadsCtx { })); if let Err(e) = result { - eprintln!("Error: {:?}", e); + eprintln!("wasi-thread-{} panicked: {:?}", wasi_thread_id, e); std::process::exit(1); } })?; From 54f52dd9e6bae58c13770e6f91204907b1270b71 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 17:11:41 -0800 Subject: [PATCH 20/33] review: remove more unused code --- crates/wasi-common/src/file.rs | 42 ---------------------------------- 1 file changed, 42 deletions(-) diff --git a/crates/wasi-common/src/file.rs b/crates/wasi-common/src/file.rs index 5f4e8d8f80ac..c799b6dcbbc5 100644 --- a/crates/wasi-common/src/file.rs +++ b/crates/wasi-common/src/file.rs @@ -3,12 +3,6 @@ use bitflags::bitflags; use std::any::Any; use std::sync::{Arc, RwLock}; -#[cfg(unix)] -use cap_std::io_lifetimes::{AsFd, BorrowedFd}; - -#[cfg(windows)] -use io_extras::os::windows::{AsRawHandleOrSocket, RawHandleOrSocket}; - #[wiggle::async_trait] pub trait WasiFile: Send + Sync { fn as_any(&self) -> &dyn Any; @@ -324,39 +318,3 @@ pub enum Advice { DontNeed, NoReuse, } - -#[cfg(unix)] -pub struct BorrowedAsFd<'a, T: AsFd>(&'a T); - -#[cfg(unix)] -impl<'a, T: AsFd> BorrowedAsFd<'a, T> { - pub fn new(t: &'a T) -> Self { - BorrowedAsFd(t) - } -} - -#[cfg(unix)] -impl AsFd for BorrowedAsFd<'_, T> { - #[inline] - fn as_fd(&self) -> BorrowedFd { - self.0.as_fd() - } -} - -#[cfg(windows)] -pub struct BorrowedAsRawHandleOrSocket<'a, T: AsRawHandleOrSocket>(&'a T); - -#[cfg(windows)] -impl<'a, T: AsRawHandleOrSocket> BorrowedAsRawHandleOrSocket<'a, T> { - pub fn new(t: &'a T) -> Self { - BorrowedAsRawHandleOrSocket(t) - } -} - -#[cfg(windows)] -impl AsRawHandleOrSocket for BorrowedAsRawHandleOrSocket<'_, T> { - #[inline] - fn as_raw_handle_or_socket(&self) -> RawHandleOrSocket { - self.0.as_raw_handle_or_socket() - } -} From 642e841b7659d07bcece4b10af518025aac93935 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 17:36:02 -0800 Subject: [PATCH 21/33] fix: change back more of #5326 --- crates/wasi-common/tokio/src/file.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/wasi-common/tokio/src/file.rs b/crates/wasi-common/tokio/src/file.rs index 0217e315b783..b8abc5485511 100644 --- a/crates/wasi-common/tokio/src/file.rs +++ b/crates/wasi-common/tokio/src/file.rs @@ -97,12 +97,12 @@ macro_rules! wasi_file_impl { self } #[cfg(unix)] - fn pollable(&self) -> Option> { - self.0.pollable() + fn pollable(&self) -> Option { + Some(self.0.as_fd()) } #[cfg(windows)] - fn pollable(&self) -> Option> { - self.0.pollable() + fn pollable(&self) -> Option { + Some(self.0.as_raw_handle_or_socket()) } async fn datasync(&self) -> Result<(), Error> { block_on_dummy_executor(|| self.0.datasync()) From e0daef966c113548831d320e696ed9beb34bd2f2 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 31 Jan 2023 19:23:55 -0800 Subject: [PATCH 22/33] fix: remove unused dependency --- crates/wasi-common/tokio/src/file.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/wasi-common/tokio/src/file.rs b/crates/wasi-common/tokio/src/file.rs index b8abc5485511..114b5a2eae34 100644 --- a/crates/wasi-common/tokio/src/file.rs +++ b/crates/wasi-common/tokio/src/file.rs @@ -6,7 +6,6 @@ use io_lifetimes::AsFd; use std::any::Any; use std::borrow::Borrow; use std::io; -use std::sync::Arc; use wasi_common::{ file::{Advice, FdFlags, FileType, Filestat, WasiFile}, Error, From 2d7107cdc2de1cf32ec7d4e63426eaf06b7e0845 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 09:24:20 -0800 Subject: [PATCH 23/33] review: conditionally `use std::sync::Arc` --- src/commands/run.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/commands/run.rs b/src/commands/run.rs index e993385e97e7..ae71ee840d54 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -12,6 +12,9 @@ use wasmtime_cli_flags::{CommonOptions, WasiModules}; use wasmtime_wasi::maybe_exit_on_error; use wasmtime_wasi::sync::{ambient_authority, Dir, TcpListener, WasiCtxBuilder}; +#[cfg(any(feature = "wasi-crypto", feature = "wasi-nn", feature = "wasi-threads"))] +use std::sync::Arc; + #[cfg(feature = "wasi-nn")] use wasmtime_wasi_nn::WasiNnCtx; @@ -416,11 +419,11 @@ impl RunCommand { struct Host { wasi: Option, #[cfg(feature = "wasi-crypto")] - wasi_crypto: Option>, + wasi_crypto: Option>, #[cfg(feature = "wasi-nn")] - wasi_nn: Option>, + wasi_nn: Option>, #[cfg(feature = "wasi-threads")] - wasi_threads: Option>>, + wasi_threads: Option>>, } /// Populates the given `Linker` with WASI APIs. @@ -469,10 +472,10 @@ fn populate_with_wasi( #[cfg(feature = "wasi-crypto")] { wasmtime_wasi_crypto::add_to_linker(linker, |host| { - std::sync::Arc::get_mut(host.wasi_crypto.as_mut().unwrap()) + Arc::get_mut(host.wasi_crypto.as_mut().unwrap()) .expect("wasi-crypto is not implemented with multi-threading support") })?; - store.data_mut().wasi_crypto = Some(std::sync::Arc::new(WasiCryptoCtx::new())); + store.data_mut().wasi_crypto = Some(Arc::new(WasiCryptoCtx::new())); } } @@ -484,10 +487,10 @@ fn populate_with_wasi( #[cfg(feature = "wasi-nn")] { wasmtime_wasi_nn::add_to_linker(linker, |host| { - std::sync::Arc::get_mut(host.wasi_nn.as_mut().unwrap()) + Arc::get_mut(host.wasi_nn.as_mut().unwrap()) .expect("wasi-nn is not implemented with multi-threading support") })?; - store.data_mut().wasi_nn = Some(std::sync::Arc::new(WasiNnCtx::new()?)); + store.data_mut().wasi_nn = Some(Arc::new(WasiNnCtx::new()?)); } } @@ -501,9 +504,9 @@ fn populate_with_wasi( wasmtime_wasi_threads::add_to_linker(linker, store, &_module, |host| { host.wasi_threads.as_ref().unwrap() })?; - store.data_mut().wasi_threads = Some(std::sync::Arc::new(WasiThreadsCtx::new( + store.data_mut().wasi_threads = Some(Arc::new(WasiThreadsCtx::new( _module, - std::sync::Arc::new(linker.clone()), + Arc::new(linker.clone()), )?)); } } From 1db99480fe47dc360f55006f12f59fead2f54760 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 09:29:15 -0800 Subject: [PATCH 24/33] review: add comments explaining `Arc::get_mut` limitations --- src/commands/run.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/commands/run.rs b/src/commands/run.rs index ae71ee840d54..b6d5a7d0720c 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -472,6 +472,13 @@ fn populate_with_wasi( #[cfg(feature = "wasi-crypto")] { wasmtime_wasi_crypto::add_to_linker(linker, |host| { + // This WASI proposal is currently not protected against + // concurrent access--i.e., when wasi-threads is actively + // spawning new threads, we cannot (yet) safely allow access and + // fail if more than one thread has `Arc`-references to the + // context. Once this proposal is updated (as wasi-common has + // been) to allow concurrent access, this `Arc::get_mut` + // limitation can be removed. Arc::get_mut(host.wasi_crypto.as_mut().unwrap()) .expect("wasi-crypto is not implemented with multi-threading support") })?; @@ -487,6 +494,7 @@ fn populate_with_wasi( #[cfg(feature = "wasi-nn")] { wasmtime_wasi_nn::add_to_linker(linker, |host| { + // See documentation for wasi-crypto for why this is needed. Arc::get_mut(host.wasi_nn.as_mut().unwrap()) .expect("wasi-nn is not implemented with multi-threading support") })?; From 87e5e6d43584b57418388019270c3d95f4f082dc Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 09:31:06 -0800 Subject: [PATCH 25/33] review: rename `_module` to `module` --- src/commands/run.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/commands/run.rs b/src/commands/run.rs index b6d5a7d0720c..1841e04ba305 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -430,7 +430,7 @@ struct Host { fn populate_with_wasi( linker: &mut Linker, store: &mut Store, - _module: Module, + module: Module, preopen_dirs: Vec<(String, Dir)>, argv: &[String], vars: &[(String, String)], @@ -509,16 +509,20 @@ fn populate_with_wasi( } #[cfg(feature = "wasi-threads")] { - wasmtime_wasi_threads::add_to_linker(linker, store, &_module, |host| { + wasmtime_wasi_threads::add_to_linker(linker, store, &module, |host| { host.wasi_threads.as_ref().unwrap() })?; store.data_mut().wasi_threads = Some(Arc::new(WasiThreadsCtx::new( - _module, + module, Arc::new(linker.clone()), )?)); } } + // Silence the unused warning for `module` as it is only used in the + // conditionally-compiled wasi-threads. + drop(&module); + Ok(()) } From e288b8b6487db26eea0ea8cbffc53cd44a7b6508 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 10:29:52 -0800 Subject: [PATCH 26/33] review: test wasi-threads in CI --- ci/run-tests.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/run-tests.sh b/ci/run-tests.sh index f81b155f2d4a..db187b779e40 100755 --- a/ci/run-tests.sh +++ b/ci/run-tests.sh @@ -2,6 +2,7 @@ cargo test \ --features "test-programs/test_programs" \ + --features wasi-threads \ --workspace \ --exclude 'wasmtime-wasi-*' \ --exclude wasi-crypto \ From 004c923398874b03875d5066a899bb6fc4172e2b Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 11:31:23 -0800 Subject: [PATCH 27/33] fix: move `drop` --- src/commands/run.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/commands/run.rs b/src/commands/run.rs index 1841e04ba305..d1ce683870c4 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -505,6 +505,10 @@ fn populate_with_wasi( if wasi_modules.wasi_threads { #[cfg(not(feature = "wasi-threads"))] { + // Silence the unused warning for `module` as it is only used in the + // conditionally-compiled wasi-threads. + drop(&module); + bail!("Cannot enable wasi-threads when the binary is not compiled with this feature."); } #[cfg(feature = "wasi-threads")] @@ -519,10 +523,6 @@ fn populate_with_wasi( } } - // Silence the unused warning for `module` as it is only used in the - // conditionally-compiled wasi-threads. - drop(&module); - Ok(()) } From ba2af1958b796ef59d8d6db65fa39edcd31825db Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 15:03:34 -0800 Subject: [PATCH 28/33] fix: hack some more iterations on the spin waiter --- tests/all/cli_tests/threads.wat | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/all/cli_tests/threads.wat b/tests/all/cli_tests/threads.wat index ca5847a60da8..a48780b56ed2 100644 --- a/tests/all/cli_tests/threads.wat +++ b/tests/all/cli_tests/threads.wat @@ -18,9 +18,10 @@ (drop (call $__wasi_thread_spawn (i32.const 0))) (drop (call $__wasi_thread_spawn (i32.const 0))) - ;; Wasmtime has no `wait/notify` yet, so we just spin to allow the threads - ;; to do their work. - (local.set $i (i32.const 2000000)) + ;; Spin to allow the threads to do their work. TODO: replace this with + ;; a loop over `wait` with `notify` instructions in the + ;; `wasi_thread_start` entry point. + (local.set $i (i32.const 4000000)) (loop $again (local.set $i (i32.sub (local.get $i) (i32.const 1))) (br_if $again (i32.gt_s (local.get $i) (i32.const 0))) From f734f0452f1d802f3da8b5f7caef4d85d7063280 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 16:39:57 -0800 Subject: [PATCH 29/33] test: use `wait`/`notify` to improve `threads.wat` test Previously, the test simply executed in a loop for some hardcoded number of iterations. This changes uses `wait` and `notify` and atomic operations to keep track of when the spawned threads are done and join on the main thread appropriately. --- tests/all/cli_tests.rs | 11 +++++++--- tests/all/cli_tests/threads.wat | 36 +++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 80bb92736180..7086ebea68dc 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -488,8 +488,13 @@ fn run_threads() -> Result<()> { wasm.path().to_str().unwrap(), ])?; - assert!(stdout.contains("Hello _start")); - assert!(stdout.contains("Hello wasi_thread_start")); - assert!(stdout.contains("Hello done")); + assert!( + stdout + == "Called _start\n\ + Running wasi_thread_start\n\ + Running wasi_thread_start\n\ + Running wasi_thread_start\n\ + Done\n" + ); Ok(()) } diff --git a/tests/all/cli_tests/threads.wat b/tests/all/cli_tests/threads.wat index a48780b56ed2..d935289738c1 100644 --- a/tests/all/cli_tests/threads.wat +++ b/tests/all/cli_tests/threads.wat @@ -4,36 +4,42 @@ (import "" "memory" (memory $shmem 1 1 shared)) (import "wasi_snapshot_preview1" "fd_write" (func $__wasi_fd_write (param i32 i32 i32 i32) (result i32))) + (import "wasi_snapshot_preview1" "proc_exit" + (func $__wasi_proc_exit (param i32))) (import "wasi" "thread_spawn" (func $__wasi_thread_spawn (param i32) (result i32))) (func (export "_start") (local $i i32) - ;; Print "Hello _start". - (call $print (i32.const 32) (i32.const 13)) + ;; Print "Called _start". + (call $print (i32.const 32) (i32.const 14)) - ;; Print "Hello wasi_thread_start" in several threads. + ;; Print "Running wasi_thread_start" in several threads. (drop (call $__wasi_thread_spawn (i32.const 0))) (drop (call $__wasi_thread_spawn (i32.const 0))) (drop (call $__wasi_thread_spawn (i32.const 0))) - ;; Spin to allow the threads to do their work. TODO: replace this with - ;; a loop over `wait` with `notify` instructions in the - ;; `wasi_thread_start` entry point. - (local.set $i (i32.const 4000000)) + ;; Wait for all the threads to notify us that they are done. (loop $again - (local.set $i (i32.sub (local.get $i) (i32.const 1))) - (br_if $again (i32.gt_s (local.get $i) (i32.const 0))) + ;; Retrieve the i32 at address 128, compare it to -1 (it should always + ;; fail) and load it atomically to check if all three threads are + ;; complete. This wait is for 1ms or until notified, whichever is first. + (drop (memory.atomic.wait32 (i32.const 128) (i32.const -1) (i64.const 1000000))) + (br_if $again (i32.lt_s (i32.atomic.load (i32.const 128)) (i32.const 3))) ) - ;; Print "Hello done". - (call $print (i32.const 64) (i32.const 11)) + ;; Print "Done". + (call $print (i32.const 64) (i32.const 5)) ) ;; A threads-enabled module must export this spec-designated entry point. (func (export "wasi_thread_start") (param $tid i32) (param $start_arg i32) - (call $print (i32.const 96) (i32.const 24)) + (call $print (i32.const 96) (i32.const 26)) + ;; After printing, we atomically increment the value at address 128 and then + ;; wake up the main thread's join loop. + (drop (i32.atomic.rmw.add (i32.const 128) (i32.const 1))) + (drop (memory.atomic.notify (i32.const 128) (i32.const 1))) ) ;; A helper function for printing ptr-len strings. @@ -50,7 +56,7 @@ ;; We still need to export the shared memory for Wiggle's sake. (export "memory" (memory $shmem)) - (data (i32.const 32) "Hello _start\0a") - (data (i32.const 64) "Hello done\0a") - (data (i32.const 96) "Hello wasi_thread_start\0a") + (data (i32.const 32) "Called _start\0a") + (data (i32.const 64) "Done\0a") + (data (i32.const 96) "Running wasi_thread_start\0a") ) From 3005167fe46044b0e0936bf063f3d3dc82050e3c Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 16:50:02 -0800 Subject: [PATCH 30/33] review: change `invalid_argument` to `not_supported` --- crates/wasi-common/src/snapshots/preview_1.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 76ead4f090f7..2d368305485d 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -197,7 +197,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .await } else { log::warn!("`fd_fdstat_set_flags` does not work with wasi-threads enabled; see https://github.com/bytecodealliance/wasmtime/issues/5643"); - Err(Error::invalid_argument()) + Err(Error::not_supported()) } } From bc9d4437074100463f8e4109fde485bf5ec8ffcb Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 17:06:00 -0800 Subject: [PATCH 31/33] review: feature-gate and update `maybe_exit_on_error` documentation --- Cargo.toml | 2 +- crates/wasi-threads/Cargo.toml | 2 +- crates/wasi/Cargo.toml | 1 + crates/wasi/src/lib.rs | 6 ++++-- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c826666461cb..33cf23dd4a0f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ wasmtime-cli-flags = { workspace = true } wasmtime-cranelift = { workspace = true } wasmtime-environ = { workspace = true } wasmtime-wast = { workspace = true } -wasmtime-wasi = { workspace = true } +wasmtime-wasi = { workspace = true, features = ["exit"] } wasmtime-wasi-crypto = { workspace = true, optional = true } wasmtime-wasi-nn = { workspace = true, optional = true } wasmtime-wasi-threads = { workspace = true, optional = true } diff --git a/crates/wasi-threads/Cargo.toml b/crates/wasi-threads/Cargo.toml index 2667587b83df..bba6e21bd178 100644 --- a/crates/wasi-threads/Cargo.toml +++ b/crates/wasi-threads/Cargo.toml @@ -17,7 +17,7 @@ log = { workspace = true } rand = "0.8" wasi-common = { workspace = true } wasmtime = { workspace = true } -wasmtime-wasi = { workspace = true } +wasmtime-wasi = { workspace = true, features = ["exit"] } [badges] maintenance = { status = "experimental" } diff --git a/crates/wasi/Cargo.toml b/crates/wasi/Cargo.toml index 73649f4afabd..f26ec10c1b9b 100644 --- a/crates/wasi/Cargo.toml +++ b/crates/wasi/Cargo.toml @@ -25,3 +25,4 @@ anyhow = { workspace = true } default = ["sync"] sync = ["wasi-cap-std-sync"] tokio = ["wasi-tokio", "wasmtime/async", "wiggle/wasmtime_async"] +exit = [] diff --git a/crates/wasi/src/lib.rs b/crates/wasi/src/lib.rs index 6661751017e0..5227f4e8993d 100644 --- a/crates/wasi/src/lib.rs +++ b/crates/wasi/src/lib.rs @@ -87,8 +87,10 @@ pub mod snapshots { /// understands the error. If the error is not an `I32Exit` or `Trap`, return /// the error back to the caller for it to decide what to do. /// -/// Note: this function is designed for CLI usage of Wasmtime; embedders of -/// Wasmtime may want different error handling. +/// Note: this function is designed for usage where it is acceptable for +/// Wasmtime failures to terminate the parent process, such as in the Wasmtime +/// CLI; this would not be suitable for use in multi-tenant embeddings. +#[cfg(feature = "exit")] pub fn maybe_exit_on_error(e: anyhow::Error) -> anyhow::Error { use std::process; use wasmtime::Trap; From a167f768bc8fae1dd4a4f3f719e28f2695c3320d Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 1 Feb 2023 17:15:00 -0800 Subject: [PATCH 32/33] review: add top-level README with embedder warning --- crates/wasi-threads/README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 crates/wasi-threads/README.md diff --git a/crates/wasi-threads/README.md b/crates/wasi-threads/README.md new file mode 100644 index 000000000000..31478778e07c --- /dev/null +++ b/crates/wasi-threads/README.md @@ -0,0 +1,12 @@ +# wasmtime-wasi-threads + +Implement the `wasi-threads` [specification] in Wasmtime. + +[specification]: https://github.com/WebAssembly/wasi-threads + +> Note: this crate is experimental and not yet suitable for use in multi-tenant +> embeddings. As specified, a trap or WASI exit in one thread must end execution +> for all threads. Due to the complexity of stopping threads, however, this +> implementation currently exits the process entirely. This will work for some +> use cases (e.g., CLI usage) but not for embedders. This warning can be removed +> once a suitable mechanism is implemented that avoids exiting the process. From f3ece48fd3410d282c22fe8648039b6948c6a653 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 7 Feb 2023 12:10:06 -0800 Subject: [PATCH 33/33] fix: remove unnecessary `mut` --- src/commands/run.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/run.rs b/src/commands/run.rs index d1ce683870c4..4f5751c3befe 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -180,7 +180,7 @@ impl RunCommand { // Read the wasm module binary either as `*.wat` or a raw binary. let module = self.load_module(linker.engine(), &self.module)?; - let mut host = Host::default(); + let host = Host::default(); let mut store = Store::new(&engine, host); populate_with_wasi( &mut linker,