Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a panic calling host functions with refs in async mode #8434

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

alexcrichton
Copy link
Member

This commit fixes a panic when a host function defined with Func::new returned GC references and was called in async mode. The logic to auto-gc before the return values go to wasm asserted that a synchronous GC was possible but the context this function is called in could be either async or sync. The fix applied in this commit is to remove the auto-gc. This means that hosts will need to explicitly GC in these situations until auto-gc is re-added back to Wasmtime.

cc #8433 as this will make the behavior consistent, but we'll want to re-add the gc behavior.

This commit fixes a panic when a host function defined with `Func::new`
returned GC references and was called in async mode. The logic to
auto-gc before the return values go to wasm asserted that a synchronous
GC was possible but the context this function is called in could be
either async or sync. The fix applied in this commit is to remove the
auto-gc. This means that hosts will need to explicitly GC in these
situations until auto-gc is re-added back to Wasmtime.

cc bytecodealliance#8433 as this will make the behavior consistent, but we'll want to
re-add the gc behavior.
@alexcrichton alexcrichton requested a review from a team as a code owner April 22, 2024 16:41
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team April 22, 2024 16:41
@alexcrichton
Copy link
Member Author

I'll also note that I plan on backporting this to the 20.0.0 release to happen today too.

@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Apr 22, 2024
@fitzgen fitzgen added this pull request to the merge queue Apr 22, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Apr 22, 2024
…lliance#8434)

This commit fixes a panic when a host function defined with `Func::new`
returned GC references and was called in async mode. The logic to
auto-gc before the return values go to wasm asserted that a synchronous
GC was possible but the context this function is called in could be
either async or sync. The fix applied in this commit is to remove the
auto-gc. This means that hosts will need to explicitly GC in these
situations until auto-gc is re-added back to Wasmtime.

cc bytecodealliance#8433 as this will make the behavior consistent, but we'll want to
re-add the gc behavior.
Merged via the queue into bytecodealliance:main with commit f3c2a0b Apr 22, 2024
21 checks passed
@alexcrichton alexcrichton deleted the fix-panic branch April 22, 2024 19:52
fitzgen pushed a commit that referenced this pull request Apr 22, 2024
…8439)

This commit fixes a panic when a host function defined with `Func::new`
returned GC references and was called in async mode. The logic to
auto-gc before the return values go to wasm asserted that a synchronous
GC was possible but the context this function is called in could be
either async or sync. The fix applied in this commit is to remove the
auto-gc. This means that hosts will need to explicitly GC in these
situations until auto-gc is re-added back to Wasmtime.

cc #8433 as this will make the behavior consistent, but we'll want to
re-add the gc behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants