Make memory growth an async function #11460
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
Member
Author
|
Note that the first commit here is from #11459 |
alexcrichton
added a commit
to alexcrichton/wasmtime
that referenced
this pull request
Aug 19, 2025
This commit is similar to bytecodealliance#11442 and bytecodealliance#11460 except it's applied to the garbage collection phase of Wasmtime's GC. Specifically the functions that actually perform a GC are no longer duplicated across sync, async, and maybe async versions. There's only one "always async" version and the root-level crate entrypoints for sync versions assert that async support is disabled and then use the async version. Worth noting here is that GC suffers from a preexisting issue described in bytecodealliance#11409 where it's not sound how a `StoreOpaque` is widened to acquire a resource limiter. This commit seemingly makes the issue worse by adding a few more `unsafe` blocks, but they're all fundamentally doing the same thing as before. Fully solving this issue will require making memory/table creation an `async` function that takes the limiter as an argument. Doing this will require further refactoring/code movement so my goal is to effectively maintain the status quo, but in a slightly different location, and enable knocking out the `unsafe` in the future. In the meantime the previous `unsafe` block is "lifted higher up" so it's not quite so deep and should be easier to remove in the future.
Member
Seems like we should call the async limiter of the store that is doing the growth, that way an embedder could still guarantee that all growth is limited from a holistic system point of view, even if one particular store's limiter doesn't handle all the grow calls. Mind filing a follow up issue for this? |
Member
Author
|
That's theoretically possible yeah but it's already preexisting that the resource limiter isn't used when a I've added a note to #4245 to accompany similar issues in #4240 and #4244 |
fitzgen
approved these changes
Aug 19, 2025
This is an analog of bytecodealliance#11442 but for memories. This had a little more impact due to memories being hooked into GC operations. Further refactoring of GC operations to make them safer/more-async is deferred to a future PR and for now it's "no worse than before". This is another step towards bytecodealliance#11430 and enables removing a longstanding `unsafe` block in `runtime/memory.rs` which previously could not be removed. One semantic change from this is that growth of a shared memory no longer uses an async limiter. This is done to keep growth of a shared memory consistent with creation of a shared memory where no limits are applied. This is due to the cross-store nature of shared memories which means that we can't tie growth to any one particular store. This additionally fixes an issue where an rwlock write guard was otherwise held across a `.await` point which creates a non-`Send` future, closing a possible soundness hole in Wasmtime.
alexcrichton
force-pushed
the
memory-growth-async
branch
from
5036f8e to
8f50983
Compare
auto-merge was automatically disabled
August 19, 2025 21:14
Pull Request is not mergeable
alexcrichton
added a commit
to alexcrichton/wasmtime
that referenced
this pull request
Aug 19, 2025
This commit is similar to bytecodealliance#11442 and bytecodealliance#11460 except it's applied to the garbage collection phase of Wasmtime's GC. Specifically the functions that actually perform a GC are no longer duplicated across sync, async, and maybe async versions. There's only one "always async" version and the root-level crate entrypoints for sync versions assert that async support is disabled and then use the async version. Worth noting here is that GC suffers from a preexisting issue described in bytecodealliance#11409 where it's not sound how a `StoreOpaque` is widened to acquire a resource limiter. This commit seemingly makes the issue worse by adding a few more `unsafe` blocks, but they're all fundamentally doing the same thing as before. Fully solving this issue will require making memory/table creation an `async` function that takes the limiter as an argument. Doing this will require further refactoring/code movement so my goal is to effectively maintain the status quo, but in a slightly different location, and enable knocking out the `unsafe` in the future. In the meantime the previous `unsafe` block is "lifted higher up" so it's not quite so deep and should be easier to remove in the future.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 19, 2025
* Make garbage collection an `async` function This commit is similar to #11442 and #11460 except it's applied to the garbage collection phase of Wasmtime's GC. Specifically the functions that actually perform a GC are no longer duplicated across sync, async, and maybe async versions. There's only one "always async" version and the root-level crate entrypoints for sync versions assert that async support is disabled and then use the async version. Worth noting here is that GC suffers from a preexisting issue described in #11409 where it's not sound how a `StoreOpaque` is widened to acquire a resource limiter. This commit seemingly makes the issue worse by adding a few more `unsafe` blocks, but they're all fundamentally doing the same thing as before. Fully solving this issue will require making memory/table creation an `async` function that takes the limiter as an argument. Doing this will require further refactoring/code movement so my goal is to effectively maintain the status quo, but in a slightly different location, and enable knocking out the `unsafe` in the future. In the meantime the previous `unsafe` block is "lifted higher up" so it's not quite so deep and should be easier to remove in the future. * Review comments * Fix configured build * More configured build fixes
bongjunj
pushed a commit
to prosyslab/wasmtime
that referenced
this pull request
Oct 20, 2025
* Make memory growth an `async` function This is an analog of bytecodealliance#11442 but for memories. This had a little more impact due to memories being hooked into GC operations. Further refactoring of GC operations to make them safer/more-async is deferred to a future PR and for now it's "no worse than before". This is another step towards bytecodealliance#11430 and enables removing a longstanding `unsafe` block in `runtime/memory.rs` which previously could not be removed. One semantic change from this is that growth of a shared memory no longer uses an async limiter. This is done to keep growth of a shared memory consistent with creation of a shared memory where no limits are applied. This is due to the cross-store nature of shared memories which means that we can't tie growth to any one particular store. This additionally fixes an issue where an rwlock write guard was otherwise held across a `.await` point which creates a non-`Send` future, closing a possible soundness hole in Wasmtime. * Fix threads-disabled build * Review comments
bongjunj
pushed a commit
to prosyslab/wasmtime
that referenced
this pull request
Oct 20, 2025
* Make garbage collection an `async` function This commit is similar to bytecodealliance#11442 and bytecodealliance#11460 except it's applied to the garbage collection phase of Wasmtime's GC. Specifically the functions that actually perform a GC are no longer duplicated across sync, async, and maybe async versions. There's only one "always async" version and the root-level crate entrypoints for sync versions assert that async support is disabled and then use the async version. Worth noting here is that GC suffers from a preexisting issue described in bytecodealliance#11409 where it's not sound how a `StoreOpaque` is widened to acquire a resource limiter. This commit seemingly makes the issue worse by adding a few more `unsafe` blocks, but they're all fundamentally doing the same thing as before. Fully solving this issue will require making memory/table creation an `async` function that takes the limiter as an argument. Doing this will require further refactoring/code movement so my goal is to effectively maintain the status quo, but in a slightly different location, and enable knocking out the `unsafe` in the future. In the meantime the previous `unsafe` block is "lifted higher up" so it's not quite so deep and should be easier to remove in the future. * Review comments * Fix configured build * More configured build fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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 is an analog of #11442 but for memories. This had a little more
impact due to memories being hooked into GC operations. Further
refactoring of GC operations to make them safer/more-async is deferred
to a future PR and for now it's "no worse than before". This is another
step towards #11430 and enables removing a longstanding
unsafeblockin
runtime/memory.rswhich previously could not be removed.One semantic change from this is that growth of a shared memory no
longer uses an async limiter. This is done to keep growth of a shared
memory consistent with creation of a shared memory where no limits are
applied. This is due to the cross-store nature of shared memories which
means that we can't tie growth to any one particular store. This
additionally fixes an issue where an rwlock write guard was otherwise
held across a
.awaitpoint which creates a non-Sendfuture, closinga possible soundness hole in Wasmtime.