Adding a way to get an unlocked WasiState mutex

[TheLostLambda]

Description

I'm currently developing a WASM-based plugin system for Mosaic, 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>> – exactly what I needed to pass a thread-safe copy of the state into my host_open_file() function

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:

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

• Add a short description of the the change to the CHANGELOG.md file

[syrusakbary]

Thanks for opening the issue! @MarkMcCaskey is probably the best person to assess the change!

[MarkMcCaskey]

Interesting use case, I hadn't considered this. Your comment about deadlocks makes me realize that it'd probably be a good idea to use a reentrant mutex here so that a single thread can lock the mutex, do stuff, call into Wasm, and have that Wasm call back into the host without deadlocking... None of the existing WASI syscalls do that but it seems like a useful pattern.

I suppose it's fine to consider Arc<Mutex<WasiState>> stable enough to expose in a public API... it may have to change in the future but I can't see another way to get access to the Wasi filesystem.

The one thing I'd like to see in this PR is a doc comment on the state field now that it's public. Perhaps something like:

/// Shared state of the WASI system. Manages all the data that the executing WASI program
/// can see.

[MarkMcCaskey]

Oh and following up about reentrant mutexes: don't worry about it in this PR, but I do think that's probably something we'd want to do before shipping 1.0 as this is now part of the public interface. I'll file an issue for it

[TheLostLambda]

Awesome :) I'll add that comment now and then we can get things merged! Hopefully I'll have a couple more PRs soonish too :) Thanks for everything you all do!

[TheLostLambda]

Oh, and I'll update the change-log quickly too!

[TheLostLambda]

Should be all sorted now :)

[MarkMcCaskey]

Thanks!

[MarkMcCaskey]

bors r+

[bors]

Build succeeded:

• Test on macos
• Test on linux
• Audit
• Code lint
• Test on linux-aarch64
• Test on windows

[MarkMcCaskey]

Following up on the reentrant mutex: I tried to implement it just now and realized some important things:

• https://docs.rs/parking_lot/0.11.1/parking_lot/type.ReentrantMutex.html can't give access to &mut because of Rust mutable aliasing rules... we could possibly work around this by implementing something that uses thread-local variables
• the reentrant mutex pattern solves some problems but causes others in multi-threaded uses

So I think the best we can reasonably do for now is document the potential for deadlocks and require users that modify the WasiState to always leave it in a valid state: they can't keep it locked and call into Wasm code.

It's unfortunate that there's no straightforward way to enforce this at the type level, but I think not using a reentrant mutex is actually simpler in the long term.