Document guidance around multithreading and Wasmtime#2812
Merged
alexcrichton merged 2 commits intoApr 7, 2021
Conversation
This commit writes a page of documentation for the Wasmtime book to serve as guidance for embedders looking to add multithreading with Wasmtime support. As always with any safe Rust API this reading is optional because you can't mis-use Wasmtime without `unsafe`, but I'm hoping that this documentation can serve as a point of reference for folks who want to add multithreading but are confused/annoyed that Wasmtime's types do not implement the `Send` and `Sync` traits. Closes bytecodealliance#793
peterhuene
reviewed
Apr 7, 2021
github-actions
Bot
added
the
wasmtime:docs
abrown
approved these changes
Apr 7, 2021
peterhuene
reviewed
Apr 7, 2021
peterhuene
reviewed
Apr 7, 2021
peterhuene
approved these changes
Apr 7, 2021
Member
peterhuene
left a comment
peterhuene
left a comment
There was a problem hiding this comment.
Looks great, just minor nits!
alexcrichton
added a commit
to alexcrichton/wasmtime
that referenced
this pull request
Apr 28, 2021
Platforms Wasmtime supports may have per-thread initialization that needs to run before WebAssembly. For example Unix needs to setup a sigaltstack and macOS needs to set up mach ports. In bytecodealliance#2757 this per-thread setup was moved out of the invocation of a wasm function, relying on the lack of Send for Store to initialize the thread at Store creation time and never worry about it later. This conflicted with [wasmtime's desired multithreading story](bytecodealliance#2812) so a new [`Store::notify_switched_thread` was added](bytecodealliance#2822) to explicitly indicate a Store has moved to another thread (if it unsafely did so). It turns out though that it's not always easy to determine when a `Store` moves to a new thread. For example the Go bindings for Wasmtime are generally unaware when a goroutine switches OS threads. This led to bytecodealliance/wasmtime-go#74 where a SIGILL was left uncaught, making it appear that traps aren't working properly. This commit revisits the decision in bytecodealliance#2757 and moves per-thread initialization back into the path of calling into WebAssembly. This is differently from before, though, where there's still only one TLS access on the path of calling into WebAssembly, unlike before where it was a separate access. This allows us to get the speed benefits of bytecodealliance#2757 as well as the flexibility benefits of not having to explicitly move a store between threads. With this new ability this commit deletes the recently added `Store::notify_switched_thread` method since it's no longer necessary.
alexcrichton
added a commit
that referenced
this pull request
Apr 28, 2021
* Bring back per-thread lazy initialization Platforms Wasmtime supports may have per-thread initialization that needs to run before WebAssembly. For example Unix needs to setup a sigaltstack and macOS needs to set up mach ports. In #2757 this per-thread setup was moved out of the invocation of a wasm function, relying on the lack of Send for Store to initialize the thread at Store creation time and never worry about it later. This conflicted with [wasmtime's desired multithreading story](#2812) so a new [`Store::notify_switched_thread` was added](#2822) to explicitly indicate a Store has moved to another thread (if it unsafely did so). It turns out though that it's not always easy to determine when a `Store` moves to a new thread. For example the Go bindings for Wasmtime are generally unaware when a goroutine switches OS threads. This led to bytecodealliance/wasmtime-go#74 where a SIGILL was left uncaught, making it appear that traps aren't working properly. This commit revisits the decision in #2757 and moves per-thread initialization back into the path of calling into WebAssembly. This is differently from before, though, where there's still only one TLS access on the path of calling into WebAssembly, unlike before where it was a separate access. This allows us to get the speed benefits of #2757 as well as the flexibility benefits of not having to explicitly move a store between threads. With this new ability this commit deletes the recently added `Store::notify_switched_thread` method since it's no longer necessary. * Fix a test compiling
mchesser
pushed a commit
to mchesser/wasmtime
that referenced
this pull request
May 24, 2021
* Bring back per-thread lazy initialization Platforms Wasmtime supports may have per-thread initialization that needs to run before WebAssembly. For example Unix needs to setup a sigaltstack and macOS needs to set up mach ports. In bytecodealliance#2757 this per-thread setup was moved out of the invocation of a wasm function, relying on the lack of Send for Store to initialize the thread at Store creation time and never worry about it later. This conflicted with [wasmtime's desired multithreading story](bytecodealliance#2812) so a new [`Store::notify_switched_thread` was added](bytecodealliance#2822) to explicitly indicate a Store has moved to another thread (if it unsafely did so). It turns out though that it's not always easy to determine when a `Store` moves to a new thread. For example the Go bindings for Wasmtime are generally unaware when a goroutine switches OS threads. This led to bytecodealliance/wasmtime-go#74 where a SIGILL was left uncaught, making it appear that traps aren't working properly. This commit revisits the decision in bytecodealliance#2757 and moves per-thread initialization back into the path of calling into WebAssembly. This is differently from before, though, where there's still only one TLS access on the path of calling into WebAssembly, unlike before where it was a separate access. This allows us to get the speed benefits of bytecodealliance#2757 as well as the flexibility benefits of not having to explicitly move a store between threads. With this new ability this commit deletes the recently added `Store::notify_switched_thread` method since it's no longer necessary. * Fix a test compiling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit writes a page of documentation for the Wasmtime book to
serve as guidance for embedders looking to add multithreading with
Wasmtime support. As always with any safe Rust API this reading is
optional because you can't mis-use Wasmtime without
unsafe, but I'mhoping that this documentation can serve as a point of reference for
folks who want to add multithreading but are confused/annoyed that
Wasmtime's types do not implement the
SendandSynctraits.Closes #793