bytecodealliance / bytecodealliance/wasmtime

`LinearMemory` thread-safety clarification

Open
#7,946 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
18.6k
Forks
1.8k
Avg merge
1d 18h
Merged PRs (30d)
126

Description

I am trying to write my own `LinearMemory` implementation. The issue that I am facing is that it appears the `as_ptr` function is inherently incompatible with the `Sync` trait bound. In essence, `as_ptr` allows interior mutability, since it's a `&self` function that returns a `mut` pointer to data owned by the memory. Interior mutability is explicitly not `Sync`, though.

Assume `T: LinearMemory`. It must also be `Sync`, so `&T` must be safely shareable between threads. However, then two threads can call `as_ptr`, _which is a safe function_, and obtain `*mut T`. At that point we're sharing mutable pointers concurrently and all bets are off. So any useful `T` cannot be `Sync` in my mind.

As far as I can grok what the runtime is doing with the memory, it only uses `LinearMemory` implementations from a single thread as long as wasm threading and `SharedMemory` is not involved. So it kinda seems like I am required to lie to the Rust compiler that my type is `Sync` and then trust `wasmtime` that it will not do anything nasty with it.

I looked at the implementation of `MmapMemory` used by the actual runtime, and underneath in `sys::unix::mmap::Mmap` you just wrap the raw pointer in `SendSyncPtr` and pretend it's `Sync`, so I am assuming that this is what would be expected from someone implementing `LinearMemory` from the outside as well.

Here are my questions:

1. First, am I even correct above or is there something I'm missing?
2. Could we get this documented in `LinearMemory` to guide implementors? For example, is it correct to assume that `as_ptr` will never be called from different threads on the same instance if wasm threads and shared memory are not involved? If yes, then at least I can rest assured that as long as I use my `T: LinearMemory` only in a single-threaded context with single-threaded `wasmtime` nothing bad will happen.
3. Can this `Sync` requirement be lifted altogether? It seems that since `LinearMemory` inherently requires interior mutability it'd make sense for it to _not_ be `Sync`, and instead have `wasmtime` handle it unsafely itself, for example by wrapping it in a type with `unsafe impl Sync` that is internal to the runtime. That way it'd guaranteed that thread madness can only happen inside `wasmtime`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.