From bb3734bd724683cce8aa20de0cad9f9086e07b50 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 5 Aug 2023 19:02:45 +0200 Subject: [PATCH] Change preview2 builder methods to use `&mut self` (#6770) * Change preview2 builder methods to use `&mut self` This commit changes the `WasiCtxBuilder` for preview2 to use a builder pattern more similar to `std::process::Command` where methods take `&mut self` and return `&mut Self` instead of taking `self` and returning `Self`. This pattern enables more easily building up configuration over time throughout code where ownership transfer might otherwise be awkward. A small caveat to this is that the ergonomics of this pattern only really work out well if the final "build" method takes `&mut self` as well. In this situation it's difficult to try to figure out what's supposed to happen if this method is called twice, so I left it to panic for now so we can more easily update it in the future possibly. * Synchronize preview1/preview2 builders * Move preview1 builders to `&mut`-style * Rename methods on preview2 builder to match names on the preview1 builders * Fix C API * Fix more tests * Fix benchmark build * Fix unused variable --- benches/wasi.rs | 45 ++++--- crates/c-api/src/wasi.rs | 44 +++--- crates/test-programs/tests/command.rs | 36 ++--- crates/test-programs/tests/reactor.rs | 2 +- .../test-programs/tests/wasi-cap-std-sync.rs | 10 +- crates/test-programs/tests/wasi-http.rs | 4 +- .../tests/wasi-preview1-host-in-preview2.rs | 12 +- .../tests/wasi-preview2-components-sync.rs | 12 +- .../tests/wasi-preview2-components.rs | 12 +- crates/test-programs/tests/wasi-tokio.rs | 10 +- crates/wasi-common/cap-std-sync/src/lib.rs | 92 ++++++++----- crates/wasi-common/tokio/src/lib.rs | 99 ++++++++------ crates/wasi/src/preview2/ctx.rs | 127 +++++++++++------- src/commands/run.rs | 27 ++-- 14 files changed, 294 insertions(+), 238 deletions(-) diff --git a/benches/wasi.rs b/benches/wasi.rs index ae8a7c8c17..41dc25e3bd 100644 --- a/benches/wasi.rs +++ b/benches/wasi.rs @@ -61,26 +61,29 @@ fn instantiate(wat: &[u8]) -> (Store, TypedFunc) { /// Build a WASI context with some actual data to retrieve. fn wasi_context() -> WasiCtx { - let wasi = WasiCtxBuilder::new(); - wasi.envs(&[ - ("a".to_string(), "b".to_string()), - ("b".to_string(), "c".to_string()), - ("c".to_string(), "d".to_string()), - ]) - .unwrap() - .args(&[ - "exe".to_string(), - "--flag1".to_string(), - "--flag2".to_string(), - "--flag3".to_string(), - "--flag4".to_string(), - ]) - .unwrap() - .preopened_dir( - wasmtime_wasi::Dir::open_ambient_dir("benches/wasi", wasmtime_wasi::ambient_authority()) + WasiCtxBuilder::new() + .envs(&[ + ("a".to_string(), "b".to_string()), + ("b".to_string(), "c".to_string()), + ("c".to_string(), "d".to_string()), + ]) + .unwrap() + .args(&[ + "exe".to_string(), + "--flag1".to_string(), + "--flag2".to_string(), + "--flag3".to_string(), + "--flag4".to_string(), + ]) + .unwrap() + .preopened_dir( + wasmtime_wasi::Dir::open_ambient_dir( + "benches/wasi", + wasmtime_wasi::ambient_authority(), + ) .unwrap(), - "/", - ) - .unwrap() - .build() + "/", + ) + .unwrap() + .build() } diff --git a/crates/c-api/src/wasi.rs b/crates/c-api/src/wasi.rs index e02063ed21..848409bcc8 100644 --- a/crates/c-api/src/wasi.rs +++ b/crates/c-api/src/wasi.rs @@ -70,17 +70,17 @@ impl wasi_config_t { pub fn into_wasi_ctx(self) -> Result { let mut builder = WasiCtxBuilder::new(); if self.inherit_args { - builder = builder.inherit_args()?; + builder.inherit_args()?; } else if !self.args.is_empty() { let args = self .args .into_iter() .map(|bytes| Ok(String::from_utf8(bytes)?)) .collect::>>()?; - builder = builder.args(&args)?; + builder.args(&args)?; } if self.inherit_env { - builder = builder.inherit_env()?; + builder.inherit_env()?; } else if !self.env.is_empty() { let env = self .env @@ -91,44 +91,50 @@ impl wasi_config_t { Ok((k, v)) }) .collect::>>()?; - builder = builder.envs(&env)?; + builder.envs(&env)?; } - builder = match self.stdin { - WasiConfigReadPipe::None => builder, - WasiConfigReadPipe::Inherit => builder.inherit_stdin(), + match self.stdin { + WasiConfigReadPipe::None => {} + WasiConfigReadPipe::Inherit => { + builder.inherit_stdin(); + } WasiConfigReadPipe::File(file) => { let file = cap_std::fs::File::from_std(file); let file = wasi_cap_std_sync::file::File::from_cap_std(file); - builder.stdin(Box::new(file)) + builder.stdin(Box::new(file)); } WasiConfigReadPipe::Bytes(binary) => { let binary = ReadPipe::from(binary); - builder.stdin(Box::new(binary)) + builder.stdin(Box::new(binary)); } }; - builder = match self.stdout { - WasiConfigWritePipe::None => builder, - WasiConfigWritePipe::Inherit => builder.inherit_stdout(), + match self.stdout { + WasiConfigWritePipe::None => {} + WasiConfigWritePipe::Inherit => { + builder.inherit_stdout(); + } WasiConfigWritePipe::File(file) => { let file = cap_std::fs::File::from_std(file); let file = wasi_cap_std_sync::file::File::from_cap_std(file); - builder.stdout(Box::new(file)) + builder.stdout(Box::new(file)); } }; - builder = match self.stderr { - WasiConfigWritePipe::None => builder, - WasiConfigWritePipe::Inherit => builder.inherit_stderr(), + match self.stderr { + WasiConfigWritePipe::None => {} + WasiConfigWritePipe::Inherit => { + builder.inherit_stderr(); + } WasiConfigWritePipe::File(file) => { let file = cap_std::fs::File::from_std(file); let file = wasi_cap_std_sync::file::File::from_cap_std(file); - builder.stderr(Box::new(file)) + builder.stderr(Box::new(file)); } }; for (dir, path) in self.preopen_dirs { - builder = builder.preopened_dir(dir, path)?; + builder.preopened_dir(dir, path)?; } for (fd_num, listener) in self.preopen_sockets { - builder = builder.preopened_socket(fd_num, listener)?; + builder.preopened_socket(fd_num, listener)?; } Ok(builder.build()) } diff --git a/crates/test-programs/tests/command.rs b/crates/test-programs/tests/command.rs index c779b998ab..7b5877e9da 100644 --- a/crates/test-programs/tests/command.rs +++ b/crates/test-programs/tests/command.rs @@ -63,7 +63,7 @@ async fn instantiate( async fn hello_stdout() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .set_args(&["gussie", "sparky", "willa"]) + .args(&["gussie", "sparky", "willa"]) .build(&mut table)?; let (mut store, command) = instantiate(get_component("hello_stdout"), CommandCtx { table, wasi }).await?; @@ -77,7 +77,7 @@ async fn hello_stdout() -> Result<()> { async fn panic() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .set_args(&[ + .args(&[ "diesel", "the", "cat", @@ -100,7 +100,7 @@ async fn panic() -> Result<()> { async fn args() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .set_args(&["hello", "this", "", "is an argument", "with 🚩 emoji"]) + .args(&["hello", "this", "", "is an argument", "with 🚩 emoji"]) .build(&mut table)?; let (mut store, command) = instantiate(get_component("args"), CommandCtx { table, wasi }).await?; @@ -156,8 +156,8 @@ async fn time() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .set_monotonic_clock(FakeMonotonicClock { now: Mutex::new(0) }) - .set_wall_clock(FakeWallClock) + .monotonic_clock(FakeMonotonicClock { now: Mutex::new(0) }) + .wall_clock(FakeWallClock) .build(&mut table)?; let (mut store, command) = @@ -173,7 +173,7 @@ async fn time() -> Result<()> { async fn stdin() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .set_stdin(MemoryInputPipe::new( + .stdin(MemoryInputPipe::new( "So rested he by the Tumtum tree".into(), )) .build(&mut table)?; @@ -191,7 +191,7 @@ async fn stdin() -> Result<()> { async fn poll_stdin() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .set_stdin(MemoryInputPipe::new( + .stdin(MemoryInputPipe::new( "So rested he by the Tumtum tree".into(), )) .build(&mut table)?; @@ -209,8 +209,8 @@ async fn poll_stdin() -> Result<()> { async fn env() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .push_env("frabjous", "day") - .push_env("callooh", "callay") + .env("frabjous", "day") + .env("callooh", "callay") .build(&mut table)?; let (mut store, command) = @@ -232,7 +232,7 @@ async fn file_read() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .push_preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/") + .preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/") .build(&mut table)?; let (mut store, command) = @@ -255,7 +255,7 @@ async fn file_append() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .push_preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/") + .preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/") .build(&mut table)?; let (mut store, command) = @@ -287,7 +287,7 @@ async fn file_dir_sync() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .push_preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/") + .preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/") .build(&mut table)?; let (mut store, command) = @@ -380,7 +380,7 @@ async fn directory_list() -> Result<()> { let wasi = WasiCtxBuilder::new() .inherit_stdout() .inherit_stderr() - .push_preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/") + .preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/") .build(&mut table)?; let (mut store, command) = @@ -432,7 +432,7 @@ async fn read_only() -> Result<()> { let mut table = Table::new(); let open_dir = Dir::open_ambient_dir(dir.path(), ambient_authority())?; let wasi = WasiCtxBuilder::new() - .push_preopened_dir(open_dir, DirPerms::READ, FilePerms::READ, "/") + .preopened_dir(open_dir, DirPerms::READ, FilePerms::READ, "/") .build(&mut table)?; let (mut store, command) = @@ -451,8 +451,8 @@ async fn stream_pollable_lifetimes() -> Result<()> { // Correct execution: should succeed let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .set_args(&["correct"]) - .set_stdin(MemoryInputPipe::new(" ".into())) + .args(&["correct"]) + .stdin(MemoryInputPipe::new(" ".into())) .build(&mut table)?; let (mut store, command) = instantiate( @@ -470,8 +470,8 @@ async fn stream_pollable_lifetimes() -> Result<()> { // Incorrect execution: should trap with a TableError::HasChildren let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .set_args(&["trap"]) - .set_stdin(MemoryInputPipe::new(" ".into())) + .args(&["trap"]) + .stdin(MemoryInputPipe::new(" ".into())) .build(&mut table)?; let (mut store, command) = instantiate( diff --git a/crates/test-programs/tests/reactor.rs b/crates/test-programs/tests/reactor.rs index 87e1a6925e..432886d9c7 100644 --- a/crates/test-programs/tests/reactor.rs +++ b/crates/test-programs/tests/reactor.rs @@ -88,7 +88,7 @@ async fn instantiate( async fn reactor_tests() -> Result<()> { let mut table = Table::new(); let wasi = WasiCtxBuilder::new() - .push_env("GOOD_DOG", "gussie") + .env("GOOD_DOG", "gussie") .build(&mut table)?; let (mut store, reactor) = diff --git a/crates/test-programs/tests/wasi-cap-std-sync.rs b/crates/test-programs/tests/wasi-cap-std-sync.rs index 3e1ba3d15c..f373b33dff 100644 --- a/crates/test-programs/tests/wasi-cap-std-sync.rs +++ b/crates/test-programs/tests/wasi-cap-std-sync.rs @@ -38,19 +38,19 @@ fn run(name: &str, inherit_stdio: bool) -> Result<()> { let mut builder = WasiCtxBuilder::new(); if inherit_stdio { - builder = builder.inherit_stdio(); + builder.inherit_stdio(); } else { - builder = builder + builder .stdout(Box::new(stdout.clone())) .stderr(Box::new(stderr.clone())); } - builder = builder.arg(name)?.arg(".")?; + builder.arg(name)?.arg(".")?; println!("preopen: {:?}", workspace); let preopen_dir = cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?; - builder = builder.preopened_dir(preopen_dir, ".")?; + builder.preopened_dir(preopen_dir, ".")?; for (var, val) in test_programs::wasi_tests_environment() { - builder = builder.env(var, val)?; + builder.env(var, val)?; } let mut store = Store::new(&ENGINE, builder.build()); diff --git a/crates/test-programs/tests/wasi-http.rs b/crates/test-programs/tests/wasi-http.rs index 0b1a4fd705..e6a5480e7d 100644 --- a/crates/test-programs/tests/wasi-http.rs +++ b/crates/test-programs/tests/wasi-http.rs @@ -76,12 +76,12 @@ pub fn run(name: &str) -> anyhow::Result<()> { wasmtime_wasi_http::add_to_linker(&mut linker, |cx: &mut Ctx| &mut cx.http)?; // Create our wasi context. - let builder = WasiCtxBuilder::new().inherit_stdio().arg(name)?; + let wasi = WasiCtxBuilder::new().inherit_stdio().arg(name)?.build(); let mut store = Store::new( &ENGINE, Ctx { - wasi: builder.build(), + wasi, http: WasiHttp::new(), }, ); diff --git a/crates/test-programs/tests/wasi-preview1-host-in-preview2.rs b/crates/test-programs/tests/wasi-preview1-host-in-preview2.rs index 2f0f958c5a..9c9e8e11ae 100644 --- a/crates/test-programs/tests/wasi-preview1-host-in-preview2.rs +++ b/crates/test-programs/tests/wasi-preview1-host-in-preview2.rs @@ -41,19 +41,17 @@ async fn run(name: &str, inherit_stdio: bool) -> Result<()> { let mut builder = WasiCtxBuilder::new(); if inherit_stdio { - builder = builder.inherit_stdio(); + builder.inherit_stdio(); } else { - builder = builder - .set_stdout(stdout.clone()) - .set_stderr(stderr.clone()); + builder.stdout(stdout.clone()).stderr(stderr.clone()); } - builder = builder.set_args(&[name, "."]); + builder.args(&[name, "."]); println!("preopen: {:?}", workspace); let preopen_dir = cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?; - builder = builder.push_preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), "."); + builder.preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), "."); for (var, val) in test_programs::wasi_tests_environment() { - builder = builder.push_env(var, val); + builder.env(var, val); } let mut table = Table::new(); diff --git a/crates/test-programs/tests/wasi-preview2-components-sync.rs b/crates/test-programs/tests/wasi-preview2-components-sync.rs index 37a03d2aa8..b618b9f403 100644 --- a/crates/test-programs/tests/wasi-preview2-components-sync.rs +++ b/crates/test-programs/tests/wasi-preview2-components-sync.rs @@ -41,19 +41,17 @@ fn run(name: &str, inherit_stdio: bool) -> Result<()> { let mut builder = WasiCtxBuilder::new(); if inherit_stdio { - builder = builder.inherit_stdio(); + builder.inherit_stdio(); } else { - builder = builder - .set_stdout(stdout.clone()) - .set_stderr(stderr.clone()); + builder.stdout(stdout.clone()).stderr(stderr.clone()); } - builder = builder.set_args(&[name, "."]); + builder.args(&[name, "."]); println!("preopen: {:?}", workspace); let preopen_dir = cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?; - builder = builder.push_preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), "."); + builder.preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), "."); for (var, val) in test_programs::wasi_tests_environment() { - builder = builder.push_env(var, val); + builder.env(var, val); } let mut table = Table::new(); diff --git a/crates/test-programs/tests/wasi-preview2-components.rs b/crates/test-programs/tests/wasi-preview2-components.rs index 947a9eea04..c74cae9002 100644 --- a/crates/test-programs/tests/wasi-preview2-components.rs +++ b/crates/test-programs/tests/wasi-preview2-components.rs @@ -41,19 +41,17 @@ async fn run(name: &str, inherit_stdio: bool) -> Result<()> { let mut builder = WasiCtxBuilder::new(); if inherit_stdio { - builder = builder.inherit_stdio(); + builder.inherit_stdio(); } else { - builder = builder - .set_stdout(stdout.clone()) - .set_stderr(stderr.clone()); + builder.stdout(stdout.clone()).stderr(stderr.clone()); } - builder = builder.set_args(&[name, "."]); + builder.args(&[name, "."]); println!("preopen: {:?}", workspace); let preopen_dir = cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?; - builder = builder.push_preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), "."); + builder.preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), "."); for (var, val) in test_programs::wasi_tests_environment() { - builder = builder.push_env(var, val); + builder.env(var, val); } let mut table = Table::new(); diff --git a/crates/test-programs/tests/wasi-tokio.rs b/crates/test-programs/tests/wasi-tokio.rs index f14f273d0f..bd673704cc 100644 --- a/crates/test-programs/tests/wasi-tokio.rs +++ b/crates/test-programs/tests/wasi-tokio.rs @@ -38,19 +38,19 @@ async fn run(name: &str, inherit_stdio: bool) -> Result<()> { let mut builder = WasiCtxBuilder::new(); if inherit_stdio { - builder = builder.inherit_stdio(); + builder.inherit_stdio(); } else { - builder = builder + builder .stdout(Box::new(stdout.clone())) .stderr(Box::new(stderr.clone())); } - builder = builder.arg(name)?.arg(".")?; + builder.arg(name)?.arg(".")?; println!("preopen: {:?}", workspace); let preopen_dir = cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?; - builder = builder.preopened_dir(preopen_dir, ".")?; + builder.preopened_dir(preopen_dir, ".")?; for (var, val) in test_programs::wasi_tests_environment() { - builder = builder.env(var, val)?; + builder.env(var, val)?; } let mut store = Store::new(&ENGINE, builder.build()); diff --git a/crates/wasi-common/cap-std-sync/src/lib.rs b/crates/wasi-common/cap-std-sync/src/lib.rs index 15a5bec35a..832c77cfa4 100644 --- a/crates/wasi-common/cap-std-sync/src/lib.rs +++ b/crates/wasi-common/cap-std-sync/src/lib.rs @@ -48,90 +48,110 @@ pub use sched::sched_ctx; use crate::net::Socket; use cap_rand::{Rng, RngCore, SeedableRng}; +use std::mem; use std::path::Path; use wasi_common::{file::FileAccessMode, table::Table, Error, WasiCtx, WasiFile}; -pub struct WasiCtxBuilder(WasiCtx); +pub struct WasiCtxBuilder { + ctx: WasiCtx, + built: bool, +} impl WasiCtxBuilder { pub fn new() -> Self { - WasiCtxBuilder(WasiCtx::new( - random_ctx(), - clocks_ctx(), - sched_ctx(), - Table::new(), - )) - } - pub fn env(mut self, var: &str, value: &str) -> Result { - self.0.push_env(var, value)?; + WasiCtxBuilder { + ctx: WasiCtx::new(random_ctx(), clocks_ctx(), sched_ctx(), Table::new()), + built: false, + } + } + pub fn env( + &mut self, + var: &str, + value: &str, + ) -> Result<&mut Self, wasi_common::StringArrayError> { + self.ctx.push_env(var, value)?; Ok(self) } - pub fn envs(mut self, env: &[(String, String)]) -> Result { + pub fn envs( + &mut self, + env: &[(String, String)], + ) -> Result<&mut Self, wasi_common::StringArrayError> { for (k, v) in env { - self.0.push_env(k, v)?; + self.ctx.push_env(k, v)?; } Ok(self) } - pub fn inherit_env(mut self) -> Result { + pub fn inherit_env(&mut self) -> Result<&mut Self, wasi_common::StringArrayError> { for (key, value) in std::env::vars() { - self.0.push_env(&key, &value)?; + self.ctx.push_env(&key, &value)?; } Ok(self) } - pub fn arg(mut self, arg: &str) -> Result { - self.0.push_arg(arg)?; + pub fn arg(&mut self, arg: &str) -> Result<&mut Self, wasi_common::StringArrayError> { + self.ctx.push_arg(arg)?; Ok(self) } - pub fn args(mut self, arg: &[String]) -> Result { + pub fn args(&mut self, arg: &[String]) -> Result<&mut Self, wasi_common::StringArrayError> { for a in arg { - self.0.push_arg(&a)?; + self.ctx.push_arg(&a)?; } Ok(self) } - pub fn inherit_args(mut self) -> Result { + pub fn inherit_args(&mut self) -> Result<&mut Self, wasi_common::StringArrayError> { for arg in std::env::args() { - self.0.push_arg(&arg)?; + self.ctx.push_arg(&arg)?; } Ok(self) } - pub fn stdin(self, f: Box) -> Self { - self.0.set_stdin(f); + pub fn stdin(&mut self, f: Box) -> &mut Self { + self.ctx.set_stdin(f); self } - pub fn stdout(self, f: Box) -> Self { - self.0.set_stdout(f); + pub fn stdout(&mut self, f: Box) -> &mut Self { + self.ctx.set_stdout(f); self } - pub fn stderr(self, f: Box) -> Self { - self.0.set_stderr(f); + pub fn stderr(&mut self, f: Box) -> &mut Self { + self.ctx.set_stderr(f); self } - pub fn inherit_stdin(self) -> Self { + pub fn inherit_stdin(&mut self) -> &mut Self { self.stdin(Box::new(crate::stdio::stdin())) } - pub fn inherit_stdout(self) -> Self { + pub fn inherit_stdout(&mut self) -> &mut Self { self.stdout(Box::new(crate::stdio::stdout())) } - pub fn inherit_stderr(self) -> Self { + pub fn inherit_stderr(&mut self) -> &mut Self { self.stderr(Box::new(crate::stdio::stderr())) } - pub fn inherit_stdio(self) -> Self { + pub fn inherit_stdio(&mut self) -> &mut Self { self.inherit_stdin().inherit_stdout().inherit_stderr() } - pub fn preopened_dir(self, dir: Dir, guest_path: impl AsRef) -> Result { + pub fn preopened_dir( + &mut self, + dir: Dir, + guest_path: impl AsRef, + ) -> Result<&mut Self, Error> { let dir = Box::new(crate::dir::Dir::from_cap_std(dir)); - self.0.push_preopened_dir(dir, guest_path)?; + self.ctx.push_preopened_dir(dir, guest_path)?; Ok(self) } - pub fn preopened_socket(self, fd: u32, socket: impl Into) -> Result { + pub fn preopened_socket( + &mut self, + fd: u32, + socket: impl Into, + ) -> Result<&mut Self, Error> { let socket: Socket = socket.into(); let file: Box = socket.into(); - self.0 + self.ctx .insert_file(fd, file, FileAccessMode::READ | FileAccessMode::WRITE); Ok(self) } - pub fn build(self) -> WasiCtx { - self.0 + pub fn build(&mut self) -> WasiCtx { + assert!(!self.built); + let WasiCtxBuilder { ctx, .. } = mem::replace(self, Self::new()); + self.built = true; + ctx } } diff --git a/crates/wasi-common/tokio/src/lib.rs b/crates/wasi-common/tokio/src/lib.rs index 250aaabb4f..a65021c19a 100644 --- a/crates/wasi-common/tokio/src/lib.rs +++ b/crates/wasi-common/tokio/src/lib.rs @@ -6,103 +6,118 @@ pub mod net; pub mod sched; pub mod stdio; -use std::future::Future; -use std::path::Path; -pub use wasi_cap_std_sync::{clocks_ctx, random_ctx}; -use wasi_common::{file::FileAccessMode, Error, Table, WasiCtx, WasiFile}; - use crate::sched::sched_ctx; pub use dir::Dir; pub use file::File; pub use net::*; +use std::future::Future; +use std::mem; +use std::path::Path; use wasi_cap_std_sync::net::Socket; +pub use wasi_cap_std_sync::{clocks_ctx, random_ctx}; +use wasi_common::{file::FileAccessMode, Error, Table, WasiCtx, WasiFile}; -pub struct WasiCtxBuilder(WasiCtx); +pub struct WasiCtxBuilder { + ctx: WasiCtx, + built: bool, +} impl WasiCtxBuilder { pub fn new() -> Self { - WasiCtxBuilder(WasiCtx::new( - random_ctx(), - clocks_ctx(), - sched_ctx(), - Table::new(), - )) - } - pub fn env(mut self, var: &str, value: &str) -> Result { - self.0.push_env(var, value)?; + WasiCtxBuilder { + ctx: WasiCtx::new(random_ctx(), clocks_ctx(), sched_ctx(), Table::new()), + built: false, + } + } + pub fn env( + &mut self, + var: &str, + value: &str, + ) -> Result<&mut Self, wasi_common::StringArrayError> { + self.ctx.push_env(var, value)?; Ok(self) } - pub fn envs(mut self, env: &[(String, String)]) -> Result { + pub fn envs( + &mut self, + env: &[(String, String)], + ) -> Result<&mut Self, wasi_common::StringArrayError> { for (k, v) in env { - self.0.push_env(k, v)?; + self.ctx.push_env(k, v)?; } Ok(self) } - pub fn inherit_env(mut self) -> Result { + pub fn inherit_env(&mut self) -> Result<&mut Self, wasi_common::StringArrayError> { for (key, value) in std::env::vars() { - self.0.push_env(&key, &value)?; + self.ctx.push_env(&key, &value)?; } Ok(self) } - pub fn arg(mut self, arg: &str) -> Result { - self.0.push_arg(arg)?; + pub fn arg(&mut self, arg: &str) -> Result<&mut Self, wasi_common::StringArrayError> { + self.ctx.push_arg(arg)?; Ok(self) } - pub fn args(mut self, arg: &[String]) -> Result { + pub fn args(&mut self, arg: &[String]) -> Result<&mut Self, wasi_common::StringArrayError> { for a in arg { - self.0.push_arg(&a)?; + self.ctx.push_arg(&a)?; } Ok(self) } - pub fn inherit_args(mut self) -> Result { + pub fn inherit_args(&mut self) -> Result<&mut Self, wasi_common::StringArrayError> { for arg in std::env::args() { - self.0.push_arg(&arg)?; + self.ctx.push_arg(&arg)?; } Ok(self) } - pub fn stdin(self, f: Box) -> Self { - self.0.set_stdin(f); + pub fn stdin(&mut self, f: Box) -> &mut Self { + self.ctx.set_stdin(f); self } - pub fn stdout(self, f: Box) -> Self { - self.0.set_stdout(f); + pub fn stdout(&mut self, f: Box) -> &mut Self { + self.ctx.set_stdout(f); self } - pub fn stderr(self, f: Box) -> Self { - self.0.set_stderr(f); + pub fn stderr(&mut self, f: Box) -> &mut Self { + self.ctx.set_stderr(f); self } - pub fn inherit_stdin(self) -> Self { + pub fn inherit_stdin(&mut self) -> &mut Self { self.stdin(Box::new(crate::stdio::stdin())) } - pub fn inherit_stdout(self) -> Self { + pub fn inherit_stdout(&mut self) -> &mut Self { self.stdout(Box::new(crate::stdio::stdout())) } - pub fn inherit_stderr(self) -> Self { + pub fn inherit_stderr(&mut self) -> &mut Self { self.stderr(Box::new(crate::stdio::stderr())) } - pub fn inherit_stdio(self) -> Self { + pub fn inherit_stdio(&mut self) -> &mut Self { self.inherit_stdin().inherit_stdout().inherit_stderr() } pub fn preopened_dir( - self, + &mut self, dir: cap_std::fs::Dir, guest_path: impl AsRef, - ) -> Result { + ) -> Result<&mut Self, Error> { let dir = Box::new(crate::dir::Dir::from_cap_std(dir)); - self.0.push_preopened_dir(dir, guest_path)?; + self.ctx.push_preopened_dir(dir, guest_path)?; Ok(self) } - pub fn preopened_socket(self, fd: u32, socket: impl Into) -> Result { + pub fn preopened_socket( + &mut self, + fd: u32, + socket: impl Into, + ) -> Result<&mut Self, Error> { let socket: Socket = socket.into(); let file: Box = socket.into(); - self.0 + self.ctx .insert_file(fd, file, FileAccessMode::READ | FileAccessMode::WRITE); Ok(self) } - pub fn build(self) -> WasiCtx { - self.0 + pub fn build(&mut self) -> WasiCtx { + assert!(!self.built); + let WasiCtxBuilder { ctx, .. } = mem::replace(self, Self::new()); + self.built = true; + ctx } } diff --git a/crates/wasi/src/preview2/ctx.rs b/crates/wasi/src/preview2/ctx.rs index e2d05ff4bb..10408fbf2d 100644 --- a/crates/wasi/src/preview2/ctx.rs +++ b/crates/wasi/src/preview2/ctx.rs @@ -1,3 +1,4 @@ +use super::clocks::host::{monotonic_clock, wall_clock}; use crate::preview2::{ clocks::{self, HostMonotonicClock, HostWallClock}, filesystem::{Dir, TableFsExt}, @@ -6,8 +7,7 @@ use crate::preview2::{ DirPerms, FilePerms, Table, }; use cap_rand::{Rng, RngCore, SeedableRng}; - -use super::clocks::host::{monotonic_clock, wall_clock}; +use std::mem; pub struct WasiCtxBuilder { stdin: Box, @@ -22,9 +22,29 @@ pub struct WasiCtxBuilder { insecure_random_seed: u128, wall_clock: Box, monotonic_clock: Box, + built: bool, } impl WasiCtxBuilder { + /// Creates a builder for a new context with default parameters set. + /// + /// The current defaults are: + /// + /// * stdin is closed + /// * stdout and stderr eat all input but it doesn't go anywhere + /// * no env vars + /// * no arguments + /// * no preopens + /// * clocks use the host implementation of wall/monotonic clocks + /// * RNGs are all initialized with random state and suitable generator + /// quality to satisfy the requirements of WASI APIs. + /// + /// These defaults can all be updated via the various builder configuration + /// methods below. + /// + /// Note that each builder can only be used once to produce a [`WasiCtx`]. + /// Invoking the [`build`](WasiCtxBuilder::build) method will panic on the + /// second attempt. pub fn new() -> Self { // For the insecure random API, use `SmallRng`, which is fast. It's // also insecure, but that's the deal here. @@ -47,129 +67,130 @@ impl WasiCtxBuilder { insecure_random_seed, wall_clock: wall_clock(), monotonic_clock: monotonic_clock(), + built: false, } } - pub fn set_stdin(mut self, stdin: impl HostInputStream + 'static) -> Self { + pub fn stdin(&mut self, stdin: impl HostInputStream + 'static) -> &mut Self { self.stdin = Box::new(stdin); self } - pub fn set_stdout(mut self, stdout: impl HostOutputStream + 'static) -> Self { + pub fn stdout(&mut self, stdout: impl HostOutputStream + 'static) -> &mut Self { self.stdout = Box::new(stdout); self } - pub fn set_stderr(mut self, stderr: impl HostOutputStream + 'static) -> Self { + pub fn stderr(&mut self, stderr: impl HostOutputStream + 'static) -> &mut Self { self.stderr = Box::new(stderr); self } - pub fn inherit_stdin(self) -> Self { - self.set_stdin(stdio::stdin()) + pub fn inherit_stdin(&mut self) -> &mut Self { + self.stdin(stdio::stdin()) } - pub fn inherit_stdout(self) -> Self { - self.set_stdout(stdio::stdout()) + pub fn inherit_stdout(&mut self) -> &mut Self { + self.stdout(stdio::stdout()) } - pub fn inherit_stderr(self) -> Self { - self.set_stderr(stdio::stderr()) + pub fn inherit_stderr(&mut self) -> &mut Self { + self.stderr(stdio::stderr()) } - pub fn inherit_stdio(self) -> Self { + pub fn inherit_stdio(&mut self) -> &mut Self { self.inherit_stdin().inherit_stdout().inherit_stderr() } - pub fn set_env(mut self, env: &[(impl AsRef, impl AsRef)]) -> Self { - self.env = env - .iter() - .map(|(k, v)| (k.as_ref().to_owned(), v.as_ref().to_owned())) - .collect(); + pub fn envs(&mut self, env: &[(impl AsRef, impl AsRef)]) -> &mut Self { + self.env.extend( + env.iter() + .map(|(k, v)| (k.as_ref().to_owned(), v.as_ref().to_owned())), + ); self } - pub fn push_env(mut self, k: impl AsRef, v: impl AsRef) -> Self { + pub fn env(&mut self, k: impl AsRef, v: impl AsRef) -> &mut Self { self.env .push((k.as_ref().to_owned(), v.as_ref().to_owned())); self } - pub fn set_args(mut self, args: &[impl AsRef]) -> Self { - self.args = args.iter().map(|a| a.as_ref().to_owned()).collect(); + pub fn args(&mut self, args: &[impl AsRef]) -> &mut Self { + self.args.extend(args.iter().map(|a| a.as_ref().to_owned())); self } - pub fn push_arg(mut self, arg: impl AsRef) -> Self { + pub fn arg(&mut self, arg: impl AsRef) -> &mut Self { self.args.push(arg.as_ref().to_owned()); self } - pub fn push_preopened_dir( - mut self, + pub fn preopened_dir( + &mut self, dir: cap_std::fs::Dir, perms: DirPerms, file_perms: FilePerms, path: impl AsRef, - ) -> Self { + ) -> &mut Self { self.preopens .push((Dir::new(dir, perms, file_perms), path.as_ref().to_owned())); self } - /// Set the generator for the secure random number generator. + /// Set the generator for the secure random number generator to the custom + /// generator specified. /// - /// This initializes the random number generator using - /// [`cap_rand::thread_rng`]. - pub fn set_secure_random(mut self) -> Self { - self.random = random::thread_rng(); - self - } - - /// Set the generator for the secure random number generator to a custom - /// generator. - /// - /// This function is usually not needed; use [`set_secure_random`] to - /// install the default generator, which is intended to be sufficient for - /// most use cases. + /// Note that contexts have a default RNG configured which is a suitable + /// generator for WASI and is configured with a random seed per-context. /// /// Guest code may rely on this random number generator to produce fresh /// unpredictable random data in order to maintain its security invariants, /// and ideally should use the insecure random API otherwise, so using any /// prerecorded or otherwise predictable data may compromise security. - /// - /// [`set_secure_random`]: Self::set_secure_random - pub fn set_secure_random_to_custom_generator( - mut self, - random: impl RngCore + Send + Sync + 'static, - ) -> Self { + pub fn secure_random(&mut self, random: impl RngCore + Send + Sync + 'static) -> &mut Self { self.random = Box::new(random); self } - pub fn set_insecure_random( - mut self, + pub fn insecure_random( + &mut self, insecure_random: impl RngCore + Send + Sync + 'static, - ) -> Self { + ) -> &mut Self { self.insecure_random = Box::new(insecure_random); self } - pub fn set_insecure_random_seed(mut self, insecure_random_seed: u128) -> Self { + pub fn insecure_random_seed(&mut self, insecure_random_seed: u128) -> &mut Self { self.insecure_random_seed = insecure_random_seed; self } - pub fn set_wall_clock(mut self, clock: impl clocks::HostWallClock + 'static) -> Self { + pub fn wall_clock(&mut self, clock: impl clocks::HostWallClock + 'static) -> &mut Self { self.wall_clock = Box::new(clock); self } - pub fn set_monotonic_clock(mut self, clock: impl clocks::HostMonotonicClock + 'static) -> Self { + pub fn monotonic_clock( + &mut self, + clock: impl clocks::HostMonotonicClock + 'static, + ) -> &mut Self { self.monotonic_clock = Box::new(clock); self } - pub fn build(self, table: &mut Table) -> Result { + /// Uses the configured context so far to construct the final `WasiCtx`. + /// + /// This will insert resources into the provided `table`. + /// + /// Note that each `WasiCtxBuilder` can only be used to "build" once, and + /// calling this method twice will panic. + /// + /// # Panics + /// + /// Panics if this method is called twice. + pub fn build(&mut self, table: &mut Table) -> Result { + assert!(!self.built); + use anyhow::Context; let Self { stdin, @@ -183,7 +204,9 @@ impl WasiCtxBuilder { insecure_random_seed, wall_clock, monotonic_clock, - } = self; + built: _, + } = mem::replace(self, Self::new()); + self.built = true; let stdin = table.push_input_stream(stdin).context("stdin")?; let stdout = table.push_output_stream(stdout).context("stdout")?; diff --git a/src/commands/run.rs b/src/commands/run.rs index 00ef5e1fea..83d65f497c 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -698,7 +698,7 @@ fn populate_with_wasi( wasmtime_wasi::add_to_linker(linker, |host| host.wasi.as_mut().unwrap())?; let mut builder = WasiCtxBuilder::new(); - builder = builder.inherit_stdio().args(argv)?; + builder.inherit_stdio().args(argv)?; for (key, value) in vars { let value = match value { @@ -706,24 +706,22 @@ fn populate_with_wasi( None => std::env::var(key) .map_err(|_| anyhow!("environment varialbe `{key}` not found"))?, }; - builder = builder.env(key, &value)?; + builder.env(key, &value)?; } let mut num_fd: usize = 3; if listenfd { - let (n, b) = ctx_set_listenfd(num_fd, builder)?; - num_fd = n; - builder = b; + num_fd = ctx_set_listenfd(num_fd, &mut builder)?; } for listener in tcplisten.drain(..) { - builder = builder.preopened_socket(num_fd as _, listener)?; + builder.preopened_socket(num_fd as _, listener)?; num_fd += 1; } for (name, dir) in preopen_dirs.into_iter() { - builder = builder.preopened_dir(dir, name)?; + builder.preopened_dir(dir, name)?; } store.data_mut().wasi = Some(builder.build()); @@ -807,20 +805,17 @@ fn populate_with_wasi( } #[cfg(not(unix))] -fn ctx_set_listenfd(num_fd: usize, builder: WasiCtxBuilder) -> Result<(usize, WasiCtxBuilder)> { - Ok((num_fd, builder)) +fn ctx_set_listenfd(num_fd: usize, _builder: &mut WasiCtxBuilder) -> Result { + Ok(num_fd) } #[cfg(unix)] -fn ctx_set_listenfd(num_fd: usize, builder: WasiCtxBuilder) -> Result<(usize, WasiCtxBuilder)> { +fn ctx_set_listenfd(mut num_fd: usize, builder: &mut WasiCtxBuilder) -> Result { use listenfd::ListenFd; - let mut builder = builder; - let mut num_fd = num_fd; - for env in ["LISTEN_FDS", "LISTEN_FDNAMES"] { if let Ok(val) = std::env::var(env) { - builder = builder.env(env, &val)?; + builder.env(env, &val)?; } } @@ -830,12 +825,12 @@ fn ctx_set_listenfd(num_fd: usize, builder: WasiCtxBuilder) -> Result<(usize, Wa if let Some(stdlistener) = listenfd.take_tcp_listener(i)? { let _ = stdlistener.set_nonblocking(true)?; let listener = TcpListener::from_std(stdlistener); - builder = builder.preopened_socket((3 + i) as _, listener)?; + builder.preopened_socket((3 + i) as _, listener)?; num_fd = 3 + i; } } - Ok((num_fd, builder)) + Ok(num_fd) } fn generate_coredump(err: &anyhow::Error, source_name: &str, coredump_path: &str) -> Result<()> {