Skip to content

Conversation

@alexcrichton
Copy link
Member

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.

@alexcrichton alexcrichton requested a review from a team as a code owner August 19, 2025 19:42
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team August 19, 2025 19:42
@alexcrichton
Copy link
Member Author

Note that this is currently built on #11460 which is in turn built on #11459

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

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.
@alexcrichton alexcrichton added this pull request to the merge queue Aug 19, 2025
Merged via the queue into bytecodealliance:main with commit 1a88c70 Aug 19, 2025
44 checks passed
@alexcrichton alexcrichton deleted the gc-async branch August 19, 2025 22:19
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants