Start reducing unsafety of ComponentInstance
#10934
Merged
Conversation
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
Work recently in the wasip3-prototyping repository has focused on reducing the amount of `unsafe` code and improving internal abstractions to facilitate this. One major thrust that's manifested in is the removal of the usage of `*mut ComponentInstance` where possible and instead using an index to safely borrow from the store to get the entire instance. This commit does not fully realize this vision just yet but lays some groundwork leading up to this. This commit specifically removes the `*mut ComponentInstance` pointers in lift/lower contexts in favor of directly storing a `wasmtime::component::Instance`. When the raw `ComponentInstance` is needed it's acquired from the `StoreOpaque` in a safe fashion that borrows the entire store for the duration of the returned borrow. This in turn required pushing instances into the store earlier during instantiation because during instantiation an instance could call out to host APIs which do lifts/lowers. There's still more work to be done to plumb this fully into libcalls and host function invocations but I wanted to upstream the wasip3 work piecemeal a chunk at a time.
alexcrichton
commented
Jun 5, 2025
Comment on lines
-979
to
-985
| // Note that this is technically unsafe due to the fact that it enables | ||
| // `mem::swap`-ing two component instances which would get all the offsets | ||
| // mixed up and cause issues. This is scoped to just this module though as a | ||
| // convenience to forward to `&mut` methods on `ComponentInstance`. | ||
| unsafe fn instance_mut(&mut self) -> &mut ComponentInstance { | ||
| &mut *self.ptr.as_ptr() | ||
| } |
There was a problem hiding this comment.
I'll note that this comment is still equally applicable today, but I don't think it's worth it trying to keep this distinction. It's just too damn useful to be able to get a safe &mut ComponentInstance and too subtle to avoid trying to do so. I opened #10933 to track a better solution here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Work recently in the wasip3-prototyping repository has focused on reducing the amount of
unsafecode and improving internal abstractions to facilitate this. One major thrust that's manifested in is the removal of the usage of*mut ComponentInstancewhere possible and instead using an index to safely borrow from the store to get the entire instance. This commit does not fully realize this vision just yet but lays some groundwork leading up to this.This commit specifically removes the
*mut ComponentInstancepointers in lift/lower contexts in favor of directly storing awasmtime::component::Instance. When the rawComponentInstanceis needed it's acquired from theStoreOpaquein a safe fashion that borrows the entire store for the duration of the returned borrow. This in turn required pushing instances into the store earlier during instantiation because during instantiation an instance could call out to host APIs which do lifts/lowers.There's still more work to be done to plumb this fully into libcalls and host function invocations but I wanted to upstream the wasip3 work piecemeal a chunk at a time.