Make WasiCtxBuilder by-ref.

This commit makes `WasiCtxBuilder` take `&mut Self` and return `&mut
Self` for its methods.  This is needed to allow for the same
(unmoved) `WasiCtxBuilder` to be used when building a WASI context.

Also fixes up the C API to remove the unnecessary `Box::from_raw` and
`forget` calls which were previously needed for the moving version of
`WasiCtxBuilder`.
This commit is contained in:
Peter Huene
2020-02-25 13:36:36 -08:00
parent 07066835db
commit fa65a14dba
4 changed files with 113 additions and 109 deletions

View File

@@ -47,20 +47,16 @@ pub unsafe extern "C" fn wasi_config_set_argv(
argc: c_int, argc: c_int,
argv: *const *const c_char, argv: *const *const c_char,
) { ) {
let mut config = Box::from_raw(config); (*config).builder.args(
config.builder = config.builder.args(
slice::from_raw_parts(argv, argc as usize) slice::from_raw_parts(argv, argc as usize)
.iter() .iter()
.map(|a| slice::from_raw_parts(*a as *const u8, CStr::from_ptr(*a).to_bytes().len())), .map(|a| slice::from_raw_parts(*a as *const u8, CStr::from_ptr(*a).to_bytes().len())),
); );
std::mem::forget(config);
} }
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn wasi_config_inherit_argv(config: *mut wasi_config_t) { pub unsafe extern "C" fn wasi_config_inherit_argv(config: *mut wasi_config_t) {
let mut config = Box::from_raw(config); (*config).builder.inherit_args();
config.builder = config.builder.inherit_args();
std::mem::forget(config);
} }
#[no_mangle] #[no_mangle]
@@ -72,23 +68,18 @@ pub unsafe extern "C" fn wasi_config_set_env(
) { ) {
let names = slice::from_raw_parts(names, envc as usize); let names = slice::from_raw_parts(names, envc as usize);
let values = slice::from_raw_parts(values, envc as usize); let values = slice::from_raw_parts(values, envc as usize);
let mut config = Box::from_raw(config);
for i in 0..envc as usize { (*config).builder.envs(
config.builder = config.builder.env( names
CStr::from_ptr(names[i]).to_bytes(), .iter()
CStr::from_ptr(values[i]).to_bytes(), .map(|p| CStr::from_ptr(*p).to_bytes())
); .zip(values.iter().map(|p| CStr::from_ptr(*p).to_bytes())),
} );
std::mem::forget(config);
} }
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn wasi_config_inherit_env(config: *mut wasi_config_t) { pub unsafe extern "C" fn wasi_config_inherit_env(config: *mut wasi_config_t) {
let mut config = Box::from_raw(config); (*config).builder.inherit_env();
config.builder = config.builder.inherit_env();
std::mem::forget(config);
} }
#[no_mangle] #[no_mangle]
@@ -101,18 +92,14 @@ pub unsafe extern "C" fn wasi_config_set_stdin(
None => return false, None => return false,
}; };
let mut config = Box::from_raw(config); (*config).builder.stdin(file);
config.builder = config.builder.stdin(file);
std::mem::forget(config);
true true
} }
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn wasi_config_inherit_stdin(config: *mut wasi_config_t) { pub unsafe extern "C" fn wasi_config_inherit_stdin(config: *mut wasi_config_t) {
let mut config = Box::from_raw(config); (*config).builder.inherit_stdin();
config.builder = config.builder.inherit_stdin();
std::mem::forget(config);
} }
#[no_mangle] #[no_mangle]
@@ -125,18 +112,14 @@ pub unsafe extern "C" fn wasi_config_set_stdout(
None => return false, None => return false,
}; };
let mut config = Box::from_raw(config); (*config).builder.stdout(file);
config.builder = config.builder.stdout(file);
std::mem::forget(config);
true true
} }
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn wasi_config_inherit_stdout(config: *mut wasi_config_t) { pub unsafe extern "C" fn wasi_config_inherit_stdout(config: *mut wasi_config_t) {
let mut config = Box::from_raw(config); (*config).builder.inherit_stdout();
config.builder = config.builder.inherit_stdout();
std::mem::forget(config);
} }
#[no_mangle] #[no_mangle]
@@ -149,18 +132,14 @@ pub unsafe extern "C" fn wasi_config_set_stderr(
None => return false, None => return false,
}; };
let mut config = Box::from_raw(config); (*config).builder.stderr(file);
config.builder = config.builder.stderr(file);
std::mem::forget(config);
true true
} }
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn wasi_config_inherit_stderr(config: *mut wasi_config_t) { pub unsafe extern "C" fn wasi_config_inherit_stderr(config: *mut wasi_config_t) {
let mut config = Box::from_raw(config); (*config).builder.inherit_stderr();
config.builder = config.builder.inherit_stderr();
std::mem::forget(config);
} }
#[no_mangle] #[no_mangle]
@@ -182,9 +161,7 @@ pub unsafe extern "C" fn wasi_config_preopen_dir(
None => return false, None => return false,
}; };
let mut config = Box::from_raw(config); (*config).builder.preopened_dir(dir, guest_path);
config.builder = config.builder.preopened_dir(dir, guest_path);
std::mem::forget(config);
true true
} }
@@ -202,7 +179,7 @@ pub unsafe extern "C" fn wasi_instance_new(
trap: *mut *mut wasm_trap_t, trap: *mut *mut wasm_trap_t,
) -> *mut wasi_instance_t { ) -> *mut wasi_instance_t {
let store = &(*store).store.borrow(); let store = &(*store).store.borrow();
let config = Box::from_raw(config); let mut config = Box::from_raw(config);
match config.builder.build() { match config.builder.build() {
Ok(ctx) => Box::into_raw(Box::new(wasi_instance_t { Ok(ctx) => Box::into_raw(Box::new(wasi_instance_t {

View File

@@ -19,12 +19,12 @@ pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> any
// Create our wasi context with pretty standard arguments/inheritance/etc. // Create our wasi context with pretty standard arguments/inheritance/etc.
// Additionally register andy preopened directories if we have them. // Additionally register andy preopened directories if we have them.
let mut builder = wasi_common::WasiCtxBuilder::new() let mut builder = wasi_common::WasiCtxBuilder::new();
.arg(bin_name)
.arg(".") builder.arg(bin_name).arg(".").inherit_stdio();
.inherit_stdio();
for (dir, file) in get_preopens(workspace)? { for (dir, file) in get_preopens(workspace)? {
builder = builder.preopened_dir(file, dir); builder.preopened_dir(file, dir);
} }
// The nonstandard thing we do with `WasiCtxBuilder` is to ensure that // The nonstandard thing we do with `WasiCtxBuilder` is to ensure that
@@ -32,7 +32,7 @@ pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> any
// where `stdin` is never ready to be read. In some CI systems, however, // where `stdin` is never ready to be read. In some CI systems, however,
// stdin is closed which causes tests to fail. // stdin is closed which causes tests to fail.
let (reader, _writer) = os_pipe::pipe()?; let (reader, _writer) = os_pipe::pipe()?;
builder = builder.stdin(reader_to_file(reader)); builder.stdin(reader_to_file(reader));
let snapshot1 = wasmtime_wasi::Wasi::new(&store, builder.build()?); let snapshot1 = wasmtime_wasi::Wasi::new(&store, builder.build()?);
let module = Module::new(&store, &data).context("failed to create wasm module")?; let module = Module::new(&store, &data).context("failed to create wasm module")?;
let imports = module let imports = module

View File

@@ -60,38 +60,38 @@ impl PendingCString {
/// A builder allowing customizable construction of `WasiCtx` instances. /// A builder allowing customizable construction of `WasiCtx` instances.
pub struct WasiCtxBuilder { pub struct WasiCtxBuilder {
fds: HashMap<wasi::__wasi_fd_t, PendingFdEntry>, fds: Option<HashMap<wasi::__wasi_fd_t, PendingFdEntry>>,
preopens: Vec<(PathBuf, File)>, preopens: Option<Vec<(PathBuf, File)>>,
args: Vec<PendingCString>, args: Option<Vec<PendingCString>>,
env: HashMap<PendingCString, PendingCString>, env: Option<HashMap<PendingCString, PendingCString>>,
} }
impl WasiCtxBuilder { impl WasiCtxBuilder {
/// Builder for a new `WasiCtx`. /// Builder for a new `WasiCtx`.
pub fn new() -> Self { pub fn new() -> Self {
let mut builder = Self { let mut fds = HashMap::new();
fds: HashMap::new(),
preopens: Vec::new(),
args: vec![],
env: HashMap::new(),
};
builder.fds.insert(0, PendingFdEntry::Thunk(FdEntry::null)); fds.insert(0, PendingFdEntry::Thunk(FdEntry::null));
builder.fds.insert(1, PendingFdEntry::Thunk(FdEntry::null)); fds.insert(1, PendingFdEntry::Thunk(FdEntry::null));
builder.fds.insert(2, PendingFdEntry::Thunk(FdEntry::null)); fds.insert(2, PendingFdEntry::Thunk(FdEntry::null));
builder Self {
fds: Some(fds),
preopens: Some(Vec::new()),
args: Some(Vec::new()),
env: Some(HashMap::new()),
}
} }
/// Add arguments to the command-line arguments list. /// Add arguments to the command-line arguments list.
/// ///
/// Arguments must be valid UTF-8 with no NUL bytes, or else `WasiCtxBuilder::build()` will fail /// Arguments must be valid UTF-8 with no NUL bytes, or else `WasiCtxBuilder::build()` will fail
/// with `Error::EILSEQ`. /// with `Error::EILSEQ`.
pub fn args<S: AsRef<[u8]>>(mut self, args: impl IntoIterator<Item = S>) -> Self { pub fn args<S: AsRef<[u8]>>(&mut self, args: impl IntoIterator<Item = S>) -> &mut Self {
self.args = args self.args
.into_iter() .as_mut()
.map(|arg| arg.as_ref().to_vec().into()) .unwrap()
.collect(); .extend(args.into_iter().map(|a| a.as_ref().to_vec().into()));
self self
} }
@@ -99,8 +99,11 @@ impl WasiCtxBuilder {
/// ///
/// Arguments must be valid UTF-8 with no NUL bytes, or else `WasiCtxBuilder::build()` will fail /// Arguments must be valid UTF-8 with no NUL bytes, or else `WasiCtxBuilder::build()` will fail
/// with `Error::EILSEQ`. /// with `Error::EILSEQ`.
pub fn arg<S: AsRef<[u8]>>(mut self, arg: S) -> Self { pub fn arg<S: AsRef<[u8]>>(&mut self, arg: S) -> &mut Self {
self.args.push(arg.as_ref().to_vec().into()); self.args
.as_mut()
.unwrap()
.push(arg.as_ref().to_vec().into());
self self
} }
@@ -108,40 +111,46 @@ impl WasiCtxBuilder {
/// ///
/// If any arguments from the host process contain invalid UTF-8, `WasiCtxBuilder::build()` will /// If any arguments from the host process contain invalid UTF-8, `WasiCtxBuilder::build()` will
/// fail with `Error::EILSEQ`. /// fail with `Error::EILSEQ`.
pub fn inherit_args(mut self) -> Self { pub fn inherit_args(&mut self) -> &mut Self {
self.args = env::args_os().map(PendingCString::OsString).collect(); let args = self.args.as_mut().unwrap();
args.clear();
args.extend(env::args_os().map(PendingCString::OsString));
self self
} }
/// Inherit stdin from the host process. /// Inherit stdin from the host process.
pub fn inherit_stdin(mut self) -> Self { pub fn inherit_stdin(&mut self) -> &mut Self {
self.fds self.fds
.as_mut()
.unwrap()
.insert(0, PendingFdEntry::Thunk(FdEntry::duplicate_stdin)); .insert(0, PendingFdEntry::Thunk(FdEntry::duplicate_stdin));
self self
} }
/// Inherit stdout from the host process. /// Inherit stdout from the host process.
pub fn inherit_stdout(mut self) -> Self { pub fn inherit_stdout(&mut self) -> &mut Self {
self.fds self.fds
.as_mut()
.unwrap()
.insert(1, PendingFdEntry::Thunk(FdEntry::duplicate_stdout)); .insert(1, PendingFdEntry::Thunk(FdEntry::duplicate_stdout));
self self
} }
/// Inherit stdout from the host process. /// Inherit stdout from the host process.
pub fn inherit_stderr(mut self) -> Self { pub fn inherit_stderr(&mut self) -> &mut Self {
self.fds self.fds
.as_mut()
.unwrap()
.insert(2, PendingFdEntry::Thunk(FdEntry::duplicate_stderr)); .insert(2, PendingFdEntry::Thunk(FdEntry::duplicate_stderr));
self self
} }
/// Inherit the stdin, stdout, and stderr streams from the host process. /// Inherit the stdin, stdout, and stderr streams from the host process.
pub fn inherit_stdio(mut self) -> Self { pub fn inherit_stdio(&mut self) -> &mut Self {
self.fds let fds = self.fds.as_mut().unwrap();
.insert(0, PendingFdEntry::Thunk(FdEntry::duplicate_stdin)); fds.insert(0, PendingFdEntry::Thunk(FdEntry::duplicate_stdin));
self.fds fds.insert(1, PendingFdEntry::Thunk(FdEntry::duplicate_stdout));
.insert(1, PendingFdEntry::Thunk(FdEntry::duplicate_stdout)); fds.insert(2, PendingFdEntry::Thunk(FdEntry::duplicate_stderr));
self.fds
.insert(2, PendingFdEntry::Thunk(FdEntry::duplicate_stderr));
self self
} }
@@ -150,10 +159,10 @@ impl WasiCtxBuilder {
/// If any environment variables from the host process contain invalid Unicode (UTF-16 for /// If any environment variables from the host process contain invalid Unicode (UTF-16 for
/// Windows, UTF-8 for other platforms), `WasiCtxBuilder::build()` will fail with /// Windows, UTF-8 for other platforms), `WasiCtxBuilder::build()` will fail with
/// `Error::EILSEQ`. /// `Error::EILSEQ`.
pub fn inherit_env(mut self) -> Self { pub fn inherit_env(&mut self) -> &mut Self {
self.env = std::env::vars_os() let env = self.env.as_mut().unwrap();
.map(|(k, v)| (k.into(), v.into())) env.clear();
.collect(); env.extend(std::env::vars_os().map(|(k, v)| (k.into(), v.into())));
self self
} }
@@ -161,8 +170,10 @@ impl WasiCtxBuilder {
/// ///
/// Environment variable keys and values must be valid UTF-8 with no NUL bytes, or else /// Environment variable keys and values must be valid UTF-8 with no NUL bytes, or else
/// `WasiCtxBuilder::build()` will fail with `Error::EILSEQ`. /// `WasiCtxBuilder::build()` will fail with `Error::EILSEQ`.
pub fn env<S: AsRef<[u8]>>(mut self, k: S, v: S) -> Self { pub fn env<S: AsRef<[u8]>>(&mut self, k: S, v: S) -> &mut Self {
self.env self.env
.as_mut()
.unwrap()
.insert(k.as_ref().to_vec().into(), v.as_ref().to_vec().into()); .insert(k.as_ref().to_vec().into(), v.as_ref().to_vec().into());
self self
} }
@@ -172,40 +183,49 @@ impl WasiCtxBuilder {
/// Environment variable keys and values must be valid UTF-8 with no NUL bytes, or else /// Environment variable keys and values must be valid UTF-8 with no NUL bytes, or else
/// `WasiCtxBuilder::build()` will fail with `Error::EILSEQ`. /// `WasiCtxBuilder::build()` will fail with `Error::EILSEQ`.
pub fn envs<S: AsRef<[u8]>, T: Borrow<(S, S)>>( pub fn envs<S: AsRef<[u8]>, T: Borrow<(S, S)>>(
mut self, &mut self,
envs: impl IntoIterator<Item = T>, envs: impl IntoIterator<Item = T>,
) -> Self { ) -> &mut Self {
self.env = envs self.env.as_mut().unwrap().extend(envs.into_iter().map(|t| {
.into_iter() let (k, v) = t.borrow();
.map(|t| { (k.as_ref().to_vec().into(), v.as_ref().to_vec().into())
let (k, v) = t.borrow(); }));
(k.as_ref().to_vec().into(), v.as_ref().to_vec().into())
})
.collect();
self self
} }
/// Provide a File to use as stdin /// Provide a File to use as stdin
pub fn stdin(mut self, file: File) -> Self { pub fn stdin(&mut self, file: File) -> &mut Self {
self.fds.insert(0, PendingFdEntry::File(file)); self.fds
.as_mut()
.unwrap()
.insert(0, PendingFdEntry::File(file));
self self
} }
/// Provide a File to use as stdout /// Provide a File to use as stdout
pub fn stdout(mut self, file: File) -> Self { pub fn stdout(&mut self, file: File) -> &mut Self {
self.fds.insert(1, PendingFdEntry::File(file)); self.fds
.as_mut()
.unwrap()
.insert(1, PendingFdEntry::File(file));
self self
} }
/// Provide a File to use as stderr /// Provide a File to use as stderr
pub fn stderr(mut self, file: File) -> Self { pub fn stderr(&mut self, file: File) -> &mut Self {
self.fds.insert(2, PendingFdEntry::File(file)); self.fds
.as_mut()
.unwrap()
.insert(2, PendingFdEntry::File(file));
self self
} }
/// Add a preopened directory. /// Add a preopened directory.
pub fn preopened_dir<P: AsRef<Path>>(mut self, dir: File, guest_path: P) -> Self { pub fn preopened_dir<P: AsRef<Path>>(&mut self, dir: File, guest_path: P) -> &mut Self {
self.preopens.push((guest_path.as_ref().to_owned(), dir)); self.preopens
.as_mut()
.unwrap()
.push((guest_path.as_ref().to_owned(), dir));
self self
} }
@@ -213,17 +233,21 @@ impl WasiCtxBuilder {
/// ///
/// If any of the arguments or environment variables in this builder cannot be converted into /// If any of the arguments or environment variables in this builder cannot be converted into
/// `CString`s, either due to NUL bytes or Unicode conversions, this returns `Error::EILSEQ`. /// `CString`s, either due to NUL bytes or Unicode conversions, this returns `Error::EILSEQ`.
pub fn build(self) -> Result<WasiCtx> { pub fn build(&mut self) -> Result<WasiCtx> {
// Process arguments and environment variables into `CString`s, failing quickly if they // Process arguments and environment variables into `CString`s, failing quickly if they
// contain any NUL bytes, or if conversion from `OsString` fails. // contain any NUL bytes, or if conversion from `OsString` fails.
let args = self let args = self
.args .args
.take()
.ok_or(Error::EINVAL)?
.into_iter() .into_iter()
.map(|arg| arg.into_utf8_cstring()) .map(|arg| arg.into_utf8_cstring())
.collect::<Result<Vec<CString>>>()?; .collect::<Result<Vec<CString>>>()?;
let env = self let env = self
.env .env
.take()
.ok_or(Error::EINVAL)?
.into_iter() .into_iter()
.map(|(k, v)| { .map(|(k, v)| {
k.into_string().and_then(|mut pair| { k.into_string().and_then(|mut pair| {
@@ -240,7 +264,7 @@ impl WasiCtxBuilder {
let mut fds: HashMap<wasi::__wasi_fd_t, FdEntry> = HashMap::new(); let mut fds: HashMap<wasi::__wasi_fd_t, FdEntry> = HashMap::new();
// Populate the non-preopen fds. // Populate the non-preopen fds.
for (fd, pending) in self.fds { for (fd, pending) in self.fds.take().ok_or(Error::EINVAL)? {
log::debug!("WasiCtx inserting ({:?}, {:?})", fd, pending); log::debug!("WasiCtx inserting ({:?}, {:?})", fd, pending);
match pending { match pending {
PendingFdEntry::Thunk(f) => { PendingFdEntry::Thunk(f) => {
@@ -255,7 +279,7 @@ impl WasiCtxBuilder {
// so we start from there. This variable is initially 2, though, because the loop // so we start from there. This variable is initially 2, though, because the loop
// immediately does the increment and check for overflow. // immediately does the increment and check for overflow.
let mut preopen_fd: wasi::__wasi_fd_t = 2; let mut preopen_fd: wasi::__wasi_fd_t = 2;
for (guest_path, dir) in self.preopens { for (guest_path, dir) in self.preopens.take().ok_or(Error::EINVAL)? {
// We do the increment at the beginning of the loop body, so that we don't overflow // We do the increment at the beginning of the loop body, so that we don't overflow
// unnecessarily if we have exactly the maximum number of file descriptors. // unnecessarily if we have exactly the maximum number of file descriptors.
preopen_fd = preopen_fd.checked_add(1).ok_or(Error::ENFILE)?; preopen_fd = preopen_fd.checked_add(1).ok_or(Error::ENFILE)?;

View File

@@ -303,22 +303,25 @@ impl ModuleRegistry {
argv: &[String], argv: &[String],
vars: &[(String, String)], vars: &[(String, String)],
) -> Result<ModuleRegistry> { ) -> Result<ModuleRegistry> {
let mut cx1 = wasi_common::WasiCtxBuilder::new() let mut cx1 = wasi_common::WasiCtxBuilder::new();
.inherit_stdio()
.args(argv) cx1.inherit_stdio().args(argv).envs(vars);
.envs(vars);
for (name, file) in preopen_dirs { for (name, file) in preopen_dirs {
cx1 = cx1.preopened_dir(file.try_clone()?, name); cx1.preopened_dir(file.try_clone()?, name);
} }
let cx1 = cx1.build()?; let cx1 = cx1.build()?;
let mut cx2 = wasi_common::old::snapshot_0::WasiCtxBuilder::new() let mut cx2 = wasi_common::old::snapshot_0::WasiCtxBuilder::new()
.inherit_stdio() .inherit_stdio()
.args(argv) .args(argv)
.envs(vars); .envs(vars);
for (name, file) in preopen_dirs { for (name, file) in preopen_dirs {
cx2 = cx2.preopened_dir(file.try_clone()?, name); cx2 = cx2.preopened_dir(file.try_clone()?, name);
} }
let cx2 = cx2.build()?; let cx2 = cx2.build()?;
Ok(ModuleRegistry { Ok(ModuleRegistry {