Repository navigation
save and restore next_switch_item for sync-to-sync calls - #14624
Merged
Merged
Conversation
We were doing this correctly for other types of calls, but somehow never thought of or tested for sync-to-sync calls 🤦. Since we're not switching fibers in this case, we need to use a `Vec` as a stack instead of the call stack, which is slightly annoying, but not a big deal. Fixes bytecodealliance#14562 Co-authored-by: Alex Crichton <alex@alexcrichton.com>
dicej
requested review from
alexcrichton and
cfallin
and removed request for
a team and
cfallin
October 9, 2026 15:49
alexcrichton
approved these changes
Oct 9, 2026
alexcrichton
left a comment
alexcrichton
left a comment
Member
There was a problem hiding this comment.
Mind updating assert_concurrent_state_empty to check this field as well?
dicej
enabled auto-merge
October 9, 2026 16:56
dicej
disabled auto-merge
October 9, 2026 17:04
dicej
disabled auto-merge
October 9, 2026 17:04
dicej
enabled auto-merge
October 9, 2026 17:07
alexcrichton
added a commit
that referenced
this pull request
Oct 9, 2026
* [50.0.x] save and restore `next_switch_item` for sync-to-sync calls We were doing this correctly for other types of calls, but somehow never thought of or tested for sync-to-sync calls 🤦. Since we're not switching fibers in this case, we need to use a `Vec` as a stack instead of the call stack, which is slightly annoying, but not a big deal. Fixes #14562 Co-authored-by: Alex Crichton <alex@alexcrichton.com> add check for `saved_next_switch_items` to `assert_concurrent_state_empty` add test from #14618, which is also addressed by the fix for #14562 * [50.0.x] Route more component adapters to the host (#14574) Prior to this commit adapters were fully compiled inline for sync<->sync adapters, but only if the lift/lowers were sync. This was incorrect when the intermediate function type was `async`, however, in a number of ways. This bug led to a number of `bail_bug!`s and incorrect execution when using the FACT-compiled adapter. The fix in this commit is to route async-typed adapted functions through the host like other async items. This additionally refactors things internally with less duplication, for example `sync_start` and `async_start` are now `start_call`, and the various cases in the trampoline compiler are consolidated into one. This changes preexisting behavior of one test, with the comment of the test adjusted from explaining the prior behavior to explaining the current behavior. --------- Co-authored-by: Alex Crichton <alex@alexcrichton.com>
Byte-Naut
pushed a commit
to Byte-Naut/wasmtime
that referenced
this pull request
Oct 10, 2026
) This commit follows up with some more regression tests from bytecodealliance#14624 which are from other bugs but fixed by the same PR.
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
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.
We were doing this correctly for other types of calls, but somehow never thought of or tested for sync-to-sync calls 🤦.
Since we're not switching fibers in this case, we need to use a
Vecas a stack instead of the call stack, which is slightly annoying, but not a big deal.Fixes #14562