Stream: git-wasmtime

Topic: wasmtime / Issue #777 The `wasmtime::Store` type should b...

Wasmtime GitHub notifications bot (Apr 29 2020 at 17:47):

alexcrichton closed Issue #777:

As proposed in #708 the Store type in the wasmtime API should be thread-safe, meaning it should implement both Send and Sync. Currently, however, it has a few items that are not thread safe and/or need audits:

• ~~The global_exports field is an Rc-protected data structure used by wasmtime internals. I don't know myself what it would take to remove this, but @sunfishcode looks like he does in https://github.com/bytecodealliance/wasmtime/pull/390~~
• The signature_cache field requires interior mutability and currently uses a RefCell. While this could be switched to a Mutex I think it would be best to avoid this altogether. I don't personally know why this field exists, @yurydelendik do you know why it's here?
• The context field has a contained compiler field which has a number of issues. Namely Rc and RefCell are not thread safe (but can be swapped for Arc/Mutex if necessary), but the underlying Compiler type has a number of trait objects, raw pointers, etc, which would need auditing. I think the solution here is going to be making Compiler itself threadsafe by redesigning it's internals.

Wasmtime GitHub notifications bot (May 08 2020 at 15:10):

DemiMarie-parity commented on Issue #777:

@alexcrichton why is this impossible?

Wasmtime GitHub notifications bot (May 08 2020 at 16:03):

alexcrichton commented on Issue #777:

@DemiMarie-parity the way things are set up right now you instantiate Instance structures into a Store. The Store keeps a handle on each Instance, so effectively a Store stores each Instance. This is done to keep each Instance alive since we don't have a GC right now to otherwise understand when there are no references to an Instance.

Instances themselves are fundamentally not Sync because operations like global get/set aren't atomic. They're ideally Send, however, but with the Rc that we're using for memory management that forces them to not be Send.

The current state of affairs is that basically Store stores enough instance-local information that it can't be shared across threads because instances can't be shared across threads. There may still be internal refactorings or possible designs to fix this, but overall our threading story isn't great right now unfortunately.

Wasmtime GitHub notifications bot (May 08 2020 at 16:24):

DemiMarie-parity commented on Issue #777:

@alexcrichton what if we used Arc instead? I imagine that reference count operations are not in hot paths.

Wasmtime GitHub notifications bot (May 08 2020 at 18:01):

alexcrichton commented on Issue #777:

Unfortunatey it's not quite as easy as that. In Rust there's no way to statically have a reference counted type that's Send but not Sync, which is morally what we'd want here. The only static solution for that is to entirely remove the reference counting, but that doesn't jive well with the memory management that we have in wasmtime today.

Failing that we'd have to invent our own solution in wasmtime which is much more invasive than just using an Arc instead of an Rc, and I'm not really sure if that's even possible much less workable.

Wasmtime GitHub notifications bot (May 08 2020 at 19:12):

DemiMarie-parity commented on Issue #777:

What would need to change to allow JIT-compiled code to be used by multiple threads simultaneously?

Wasmtime GitHub notifications bot (May 08 2020 at 19:15):

DemiMarie-parity commented on Issue #777:

The first idea that comes to mind is to separate the JIT-compiled code (which is immutable) from the data (which is mutable). So to execute some code, one would first need to compile it, which would return an immutable CompiledBlob object. One would then need to create an Instance from the CompiledBlob. Since a CompiledBlob is immutable, making it Send + Sync is trivial.

Wasmtime GitHub notifications bot (May 08 2020 at 19:20):

DemiMarie-parity deleted a comment on Issue #777:

The first idea that comes to mind is to separate the JIT-compiled code (which is immutable) from the data (which is mutable). So to execute some code, one would first need to compile it, which would return an immutable CompiledBlob object. One would then need to create an Instance from the CompiledBlob. Since a CompiledBlob is immutable, making it Send + Sync is trivial.

Wasmtime GitHub notifications bot (May 11 2020 at 17:43):

alexcrichton commented on Issue #777:

Ah yes so sharing Store is its own issue but sharing compiled code is definitely something that we want to enable. Making Module both Send and Sync should be easier than making Store either Send or Sync, and that's largely just a matter of internal refactorings.

Wasmtime GitHub notifications bot (May 11 2020 at 23:09):

yurydelendik commented on Issue #777:

FWIW wasm-c-api planed to shared a module between two stores via wasm_module_share/wasm_module_obtain , see https://github.com/WebAssembly/wasm-c-api/blob/master/example/threads.c#L34

Last updated: Nov 22 2024 at 16:03 UTC