Skip to content

Commit

Permalink
Merge #1810
Browse files Browse the repository at this point in the history
1810: Adding a way to get an unlocked `WasiState` mutex r=MarkMcCaskey a=TheLostLambda

# Description
I'm currently developing a WASM-based [plugin system for Mosaic](https://github.com/mosaic-org/wasm-module-experiments), but I've encountered a number of rough edges throughout my time working with the 1.0 alphas (as is to be expected 🙂 ). I hope to eventually upstream a number of tweaks, but this first one is quite simple. Unfortunately, it's also something I needed a fork of the library to pull off.

I have a host-function (that can be called from the WASM module) that needs access to the stdout of the WASM VM, but I was struggling to pass the VM state to the host-function using `Function::new_native_with_env()`.

Conveniently, `state` in `WasiEnv` [is stored as an `Arc<Mutex<WasiState>>`](https://github.com/wasmerio/wasmer/blob/e9b280b4ddf290c1b27a27fa1e78114708c5fdaa/lib/wasi/src/lib.rs#L53) – exactly what I needed to pass a thread-safe copy of the state into [my `host_open_file()` function](https://github.com/mosaic-org/wasm-module-experiments/blob/e2da74fb1db5692aeb0b3a19f633b3ad3410a96c/loader/src/main.rs#L133)

Tragically, that `state` field is private, and there is no way (so far as I can see) to get a copy of that unlocked Mutex.

Just making the field public (as in this PR) might not be ideal if it's likely to change though, so I'm also open to writing a getter-function instead.

As a side note, I think that I managed to hit a deadlock when I called a WASM function in the same scope as a `WasiEnv::state()` call. This, for example, totally deadlocks the program:

![image](https://user-images.githubusercontent.com/6251883/98750168-26169900-23b5-11eb-85bf-122882b37f88.png)

This isn't really a massive issue, but would be clearer if `WasiEnv::state()` returned the actual Mutex that the user needs to lock. At the moment, this seems like an easy trap to fall into.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Brooks J Rady <[email protected]>
Co-authored-by: Brooks Rady <[email protected]>
  • Loading branch information
bors[bot] and TheLostLambda authored Nov 18, 2020
2 parents 8a68bb6 + bc72c20 commit 2b8afdc
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

### Changed

- [#1810](https://github.com/wasmerio/wasmer/pull/1810) Make the `state` field of `WasiEnv` public

### Fixed

- [#1764](https://github.com/wasmerio/wasmer/pull/1764) Fix bug in WASI `path_rename` allowing renamed files to be 1 directory below a preopened directory.
Expand Down
4 changes: 3 additions & 1 deletion lib/wasi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ pub enum WasiError {
/// The environment provided to the WASI imports.
#[derive(Debug, Clone)]
pub struct WasiEnv {
state: Arc<Mutex<WasiState>>,
/// Shared state of the WASI system. Manages all the data that the
/// executing WASI program can see.
pub state: Arc<Mutex<WasiState>>,
memory: Arc<WasiMemory>,
}

Expand Down

0 comments on commit 2b8afdc

Please sign in to comment.