Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow different Handles to act as stdio #1600

Merged
merged 6 commits into from
Jun 9, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 70 additions & 47 deletions crates/c-api/src/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,62 +201,85 @@ enum WasiInstance {
Snapshot0(WasiSnapshot0),
}

macro_rules! config_to_builder {
($builder:ident, $config:ident) => {{
let mut builder = $builder::new();

if $config.inherit_args {
builder.inherit_args();
} else if !$config.args.is_empty() {
builder.args($config.args);
}

if $config.inherit_env {
builder.inherit_env();
} else if !$config.env.is_empty() {
builder.envs($config.env);
}

if $config.inherit_stdin {
builder.inherit_stdin();
} else if let Some(file) = $config.stdin {
builder.stdin(file);
}

if $config.inherit_stdout {
builder.inherit_stdout();
} else if let Some(file) = $config.stdout {
builder.stdout(file);
}

if $config.inherit_stderr {
builder.inherit_stderr();
} else if let Some(file) = $config.stderr {
builder.stderr(file);
}

for preopen in $config.preopens {
builder.preopened_dir(preopen.0, preopen.1);
}

builder
}};
}

fn create_snapshot0_instance(store: &Store, config: wasi_config_t) -> Result<WasiInstance, String> {
let mut builder = WasiSnapshot0CtxBuilder::new();
if config.inherit_args {
builder.inherit_args();
} else if !config.args.is_empty() {
builder.args(config.args);
}
if config.inherit_env {
builder.inherit_env();
} else if !config.env.is_empty() {
builder.envs(config.env);
}
if config.inherit_stdin {
builder.inherit_stdin();
} else if let Some(file) = config.stdin {
builder.stdin(file);
}
if config.inherit_stdout {
builder.inherit_stdout();
} else if let Some(file) = config.stdout {
builder.stdout(file);
}
if config.inherit_stderr {
builder.inherit_stderr();
} else if let Some(file) = config.stderr {
builder.stderr(file);
}
for preopen in config.preopens {
builder.preopened_dir(preopen.0, preopen.1);
}
Ok(WasiInstance::Snapshot0(WasiSnapshot0::new(
store,
config_to_builder!(WasiSnapshot0CtxBuilder, config)
.build()
.map_err(|e| e.to_string())?,
builder.build().map_err(|e| e.to_string())?,
)))
}

fn wasi_preview_builder(config: wasi_config_t) -> Result<WasiPreview1CtxBuilder> {
Copy link
Member

Choose a reason for hiding this comment

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

Should this function be merged with create_preview1_instance like create_snapshot0_instance is implemented?

use std::convert::TryFrom;
use wasi_common::OsFile;
let mut builder = WasiPreview1CtxBuilder::new();
if config.inherit_args {
builder.inherit_args();
} else if !config.args.is_empty() {
builder.args(config.args);
}
if config.inherit_env {
builder.inherit_env();
} else if !config.env.is_empty() {
builder.envs(config.env);
}
if config.inherit_stdin {
builder.inherit_stdin();
} else if let Some(file) = config.stdin {
builder.stdin(OsFile::try_from(file)?);
}
if config.inherit_stdout {
builder.inherit_stdout();
} else if let Some(file) = config.stdout {
builder.stdout(OsFile::try_from(file)?);
}
if config.inherit_stderr {
builder.inherit_stderr();
} else if let Some(file) = config.stderr {
builder.stderr(OsFile::try_from(file)?);
}
for preopen in config.preopens {
builder.preopened_dir(preopen.0, preopen.1);
}
Ok(builder)
}

fn create_preview1_instance(store: &Store, config: wasi_config_t) -> Result<WasiInstance, String> {
Ok(WasiInstance::Preview1(WasiPreview1::new(
store,
config_to_builder!(WasiPreview1CtxBuilder, config)
.build()
wasi_preview_builder(config)
.and_then(|mut b| {
let b = b.build()?;
Ok(b)
})
.map_err(|e| e.to_string())?,
)))
}
Expand Down
7 changes: 5 additions & 2 deletions crates/test-programs/tests/wasm_tests/runtime.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use anyhow::Context;
use std::convert::TryFrom;
use std::fs::File;
use std::path::Path;
use wasi_common::VirtualDirEntry;
use wasi_common::{OsOther, VirtualDirEntry};
use wasmtime::{Linker, Module, Store};

#[derive(Clone, Copy, Debug)]
Expand Down Expand Up @@ -46,7 +47,9 @@ pub fn instantiate(
// where `stdin` is never ready to be read. In some CI systems, however,
// stdin is closed which causes tests to fail.
let (reader, _writer) = os_pipe::pipe()?;
builder.stdin(reader_to_file(reader));
let file = reader_to_file(reader);
let handle = OsOther::try_from(file).context("failed to create OsOther from PipeReader")?;
builder.stdin(handle);
let snapshot1 = wasmtime_wasi::Wasi::new(&store, builder.build()?);

let mut linker = Linker::new(&store);
Expand Down
27 changes: 13 additions & 14 deletions crates/wasi-common/src/ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type WasiCtxBuilderResult<T> = std::result::Result<T, WasiCtxBuilderError>;

enum PendingEntry {
Thunk(fn() -> io::Result<Box<dyn Handle>>),
OsHandle(File),
Handle(Box<dyn Handle>),
}

impl std::fmt::Debug for PendingEntry {
Expand All @@ -58,7 +58,7 @@ impl std::fmt::Debug for PendingEntry {
"PendingEntry::Thunk({:p})",
f as *const fn() -> io::Result<Box<dyn Handle>>
),
Self::OsHandle(f) => write!(fmt, "PendingEntry::OsHandle({:?})", f),
Self::Handle(handle) => write!(fmt, "PendingEntry::Handle({:p})", handle),
}
}
}
Expand Down Expand Up @@ -247,21 +247,21 @@ impl WasiCtxBuilder {
self
}

/// Provide a File to use as stdin
pub fn stdin(&mut self, file: File) -> &mut Self {
self.stdin = Some(PendingEntry::OsHandle(file));
/// Provide a `Handle` to use as stdin
pub fn stdin<T: Handle + 'static>(&mut self, handle: T) -> &mut Self {
self.stdin = Some(PendingEntry::Handle(Box::new(handle)));
self
}

/// Provide a File to use as stdout
pub fn stdout(&mut self, file: File) -> &mut Self {
self.stdout = Some(PendingEntry::OsHandle(file));
/// Provide a `Handle` to use as stdout
pub fn stdout<T: Handle + 'static>(&mut self, handle: T) -> &mut Self {
self.stdout = Some(PendingEntry::Handle(Box::new(handle)));
self
}

/// Provide a File to use as stderr
pub fn stderr(&mut self, file: File) -> &mut Self {
self.stderr = Some(PendingEntry::OsHandle(file));
/// Provide a `Handle` to use as stderr
pub fn stderr<T: Handle + 'static>(&mut self, handle: T) -> &mut Self {
self.stderr = Some(PendingEntry::Handle(Box::new(handle)));
self
}

Expand Down Expand Up @@ -368,9 +368,8 @@ impl WasiCtxBuilder {
.insert(entry)
.ok_or(WasiCtxBuilderError::TooManyFilesOpen)?
}
PendingEntry::OsHandle(f) => {
let handle = OsOther::try_from(f)?;
let handle = EntryHandle::new(handle);
PendingEntry::Handle(handle) => {
let handle = EntryHandle::from(handle);
let entry = Entry::new(handle);
entries
.insert(entry)
Expand Down
1 change: 1 addition & 0 deletions crates/wasi-common/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::rc::Rc;
pub(crate) struct EntryHandle(Rc<dyn Handle>);

impl EntryHandle {
#[allow(dead_code)]
pub(crate) fn new<T: Handle + 'static>(handle: T) -> Self {
Self(Rc::new(handle))
}
Expand Down
30 changes: 21 additions & 9 deletions crates/wasi-common/src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,38 +6,49 @@ use std::io::{self, SeekFrom};

/// Represents rights of a `Handle`, either already held or required.
#[derive(Debug, Copy, Clone)]
pub(crate) struct HandleRights {
pub struct HandleRights {
pub(crate) base: Rights,
pub(crate) inheriting: Rights,
}

impl HandleRights {
pub(crate) fn new(base: Rights, inheriting: Rights) -> Self {
/// Creates new `HandleRights` instance from `base` and `inheriting` rights.
pub fn new(base: Rights, inheriting: Rights) -> Self {
Self { base, inheriting }
}

/// Create new `HandleRights` instance from `base` rights only, keeping
/// Creates new `HandleRights` instance from `base` rights only, keeping
/// `inheriting` set to none.
pub(crate) fn from_base(base: Rights) -> Self {
pub fn from_base(base: Rights) -> Self {
Self {
base,
inheriting: Rights::empty(),
}
}

/// Create new `HandleRights` instance with both `base` and `inheriting`
/// Creates new `HandleRights` instance with both `base` and `inheriting`
/// rights set to none.
pub(crate) fn empty() -> Self {
pub fn empty() -> Self {
Self {
base: Rights::empty(),
inheriting: Rights::empty(),
}
}

/// Check if `other` is a subset of those rights.
pub(crate) fn contains(&self, other: &Self) -> bool {
/// Checks if `other` is a subset of those rights.
pub fn contains(&self, other: &Self) -> bool {
self.base.contains(&other.base) && self.inheriting.contains(&other.inheriting)
}

/// Returns base rights.
pub fn base(&self) -> Rights {
self.base
}

/// Returns inheriting rights.
pub fn inheriting(&self) -> Rights {
self.inheriting
}
}

impl fmt::Display for HandleRights {
Expand All @@ -50,7 +61,8 @@ impl fmt::Display for HandleRights {
}
}

pub(crate) trait Handle {
// TODO docs
pub trait Handle {
fn as_any(&self) -> &dyn Any;
fn try_clone(&self) -> io::Result<Box<dyn Handle>>;
fn get_file_type(&self) -> types::Filetype;
Expand Down
4 changes: 4 additions & 0 deletions crates/wasi-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,9 @@ mod virtfs;
pub mod wasi;

pub use ctx::{WasiCtx, WasiCtxBuilder, WasiCtxBuilderError};
pub use handle::{Handle, HandleRights};
pub use sys::osdir::OsDir;
pub use sys::osfile::OsFile;
pub use sys::osother::{OsOther, OsOtherExt};
pub use sys::preopen_dir;
pub use virtfs::{FileContents, VirtualDirEntry};
2 changes: 1 addition & 1 deletion crates/wasi-common/src/sys/osdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::ops::Deref;
// TODO could this be cleaned up?
// The actual `OsDir` struct is OS-dependent, therefore we delegate
// its definition to OS-specific modules.
pub(crate) use super::sys_impl::osdir::OsDir;
pub use super::sys_impl::osdir::OsDir;

impl Deref for OsDir {
type Target = RawOsHandle;
Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-common/src/sys/osfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::io::{self, Read, Seek, SeekFrom, Write};
use std::ops::Deref;

#[derive(Debug)]
pub(crate) struct OsFile {
pub struct OsFile {
rights: Cell<HandleRights>,
handle: RawOsHandle,
}
Expand Down
4 changes: 2 additions & 2 deletions crates/wasi-common/src/sys/osother.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::fs::File;
use std::io::{self, Read, Write};
use std::ops::Deref;

pub(crate) trait OsOtherExt {
pub trait OsOtherExt {
/// Create `OsOther` as `dyn Handle` from null device.
fn from_null() -> io::Result<Box<dyn Handle>>;
}
Expand All @@ -21,7 +21,7 @@ pub(crate) trait OsOtherExt {
/// pipe should be encapsulated within this instance _and not_ `OsFile` which represents a regular
/// OS file.
#[derive(Debug)]
pub(crate) struct OsOther {
pub struct OsOther {
file_type: Filetype,
rights: Cell<HandleRights>,
handle: RawOsHandle,
Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-common/src/sys/unix/bsd/osdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::io;
use yanix::dir::Dir;

#[derive(Debug)]
pub(crate) struct OsDir {
pub struct OsDir {
pub(crate) rights: Cell<HandleRights>,
pub(crate) handle: RawOsHandle,
// When the client makes a `fd_readdir` syscall on this descriptor,
Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-common/src/sys/unix/linux/osdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::io;
use yanix::dir::Dir;

#[derive(Debug)]
pub(crate) struct OsDir {
pub struct OsDir {
pub(crate) rights: Cell<HandleRights>,
pub(crate) handle: RawOsHandle,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-common/src/sys/unix/osdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::fs::File;
use std::io;
use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd};

pub(crate) use super::sys_impl::osdir::OsDir;
pub use super::sys_impl::osdir::OsDir;

impl TryFrom<File> for OsDir {
type Error = io::Error;
Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-common/src/sys/unix/oshandle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::io;
use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd};

#[derive(Debug)]
pub(crate) struct RawOsHandle(File);
pub struct RawOsHandle(File);

impl RawOsHandle {
/// Tries clone `self`.
Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-common/src/sys/windows/osdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::io;
use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle};

#[derive(Debug)]
pub(crate) struct OsDir {
pub struct OsDir {
pub(crate) rights: Cell<HandleRights>,
pub(crate) handle: RawOsHandle,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-common/src/sys/windows/oshandle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::mem::ManuallyDrop;
use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle};

#[derive(Debug)]
pub(crate) struct RawOsHandle(Cell<RawHandle>);
pub struct RawOsHandle(Cell<RawHandle>);

impl RawOsHandle {
/// Tries cloning `self`.
Expand Down