Stream: git-wasmtime

Topic: wasmtime / PR #14418 component: keep guest fibers in thei...


view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 06:10):

Byte-Naut opened PR #14418 from Byte-Naut:issue-14241-2 to bytecodealliance:main:

Fixes #14241.

This supersedes #14385 and follows @alexcrichton's suggestion there: rather than guarding each handoff, keep fibers at rest in the table and only take them out while they're being resumed.

Motivation

Several scheduler paths in concurrent.rs moved a live fiber out of store-owned state (into a WaitMode, a WorkItem, or a local) and then ran a fallible step before it reached its next owner. These only fail once an internal invariant is already broken, but when they do the unfinished fiber is dropped, StoreFiber's Drop panics, and a trap or bail_bug! turns into a host panic or process abort. #14382 added two more such paths (next_switch_item in YieldingToSubtask, and the other arm in subtask_cancel).

Changes

No public API changes, no new locks, no per-suspension allocations, and no new ConcurrentState fields. About 270 of the added lines are tests.

Design

FiberKind keeps the distinctions the thread intrinsics relied on when the fiber's location implied the state:

Before Fiber kept in Now
Running, blocked in a waitable set WaitMode::Fiber FiberKind::Waiting
Running, queued as ResumeFiber or yielding to a subtask work item / next_switch_item FiberKind::Scheduled
Suspended thread state FiberKind::Suspended
Ready, queued as ResumeThread thread state FiberKind::Ready

GuestThreadState::can_resume applies the same rules as before: only Suspended (or a not-yet-started explicit thread) can be resumed, and only Ready can be promoted. Waiting and Scheduled behave like Running. resume_work_item_fiber checks that a ResumeFiber item finds Scheduled and a ResumeThread item finds Ready before taking the fiber.

Two things worth a look in review:

Testing

thread-wait-resume.wast (new) blocks a thread in waitable-set.wait with no pending event, then calls thread.resume-later or thread.suspend-then-resume on it and expects cannot resume thread which is not suspended. It guards against a waiting thread being treated as suspended now that its fiber lives in the thread state.

The unit tests in concurrent.rs (fiber_ownership_tests) set up broken states directly: invalid wait set, thread, or subtask handles when a fiber suspends; an invalid waiter in mark_ready; an occupied switch slot; set_thread_running and cleanup_thread on a thread that still owns a fiber; and dropping a resume future before it's polled. Each checks that the original error (or bail_bug! panic in debug builds) comes back and that the fiber is still owned by the store or has been disposed of. One more test checks the resume and promote rules for each FiberKind.

cargo test -p wasmtime --lib fiber_ownership_tests
cargo test --test wast -- component-model/async

Locally: fmt, clippy for wasmtime, the full wasmtime lib tests, the async component-model .wast tests on Cranelift and Winch, the component_model tests in tests/all, the focused tests with debug assertions disabled, and no_std / non-async builds all pass.

Developed with assistance from Claude (Anthropic). Per the Bytecode Alliance AI Tool Use Policy, I'm the author and accountable for this change.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 06:10):

Byte-Naut requested cfallin for a review on PR #14418.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 06:10):

Byte-Naut requested wasmtime-core-reviewers for a review on PR #14418.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 07:48):

github-actions[bot] added the label wasmtime:api on PR #14418.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 22:11):

Byte-Naut edited PR #14418:

Fixes #14241.

This supersedes #14385 and follows @alexcrichton's suggestion there: rather than guarding each handoff, keep fibers at rest in the table and only take them out while they're being resumed.

Motivation

Several scheduler paths in concurrent.rs moved a live fiber out of store-owned state (into a WaitMode, a WorkItem, or a local) and then ran a fallible step before it reached its next owner. These only fail once an internal invariant is already broken, but when they do the unfinished fiber is dropped, StoreFiber's Drop panics, and a trap or bail_bug! turns into a host panic or process abort. #14382 added two more such paths (next_switch_item in YieldingToSubtask, and the other arm in subtask_cancel).

Changes

No public API changes, no new locks, no per-suspension allocations, and no new ConcurrentState fields. About 270 of the added lines are tests.

Design

FiberKind keeps the distinctions the thread intrinsics relied on when the fiber's location implied the state:

Before Fiber kept in Now
Running, blocked in a waitable set WaitMode::Fiber FiberKind::Waiting
Running, queued as ResumeFiber or yielding to a subtask work item / next_switch_item FiberKind::Scheduled
Suspended thread state FiberKind::Suspended
Ready, queued as ResumeThread thread state FiberKind::Ready

GuestThreadState::can_resume applies the same rules as before: only Suspended (or a not-yet-started explicit thread) can be resumed, and only Ready can be promoted. Waiting and Scheduled behave like Running. resume_work_item_fiber checks that a ResumeFiber item finds Scheduled and a ResumeThread item finds Ready before taking the fiber.

Two things worth a look in review:

None of the details here are fixed. If other names for FiberKind's variants, or fewer or differently placed tests, would be easier to review, I'm happy to rework it.

Testing

thread-wait-resume.wast (new) blocks a thread in waitable-set.wait with no pending event, then calls thread.resume-later or thread.suspend-then-resume on it and expects cannot resume thread which is not suspended. It guards against a waiting thread being treated as suspended now that its fiber lives in the thread state.

The unit tests in concurrent.rs (fiber_ownership_tests) set up broken states directly: invalid wait set, thread, or subtask handles when a fiber suspends; an invalid waiter in mark_ready; an occupied switch slot; set_thread_running and cleanup_thread on a thread that still owns a fiber; and dropping a resume future before it's polled. Each checks that the original error (or bail_bug! panic in debug builds) comes back and that the fiber is still owned by the store or has been disposed of. One more test checks the resume and promote rules for each FiberKind.

cargo test -p wasmtime --lib fiber_ownership_tests
cargo test --test wast -- component-model/async

Locally: fmt, clippy for wasmtime, the full wasmtime lib tests, the async component-model .wast tests on Cranelift and Winch, the component_model tests in tests/all, the focused tests with debug assertions disabled, and no_std / non-async builds all pass.

Developed with assistance from Claude (Anthropic). Per the Bytecode Alliance AI Tool Use Policy, I'm the author and accountable for this change.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 26 2026 at 00:19):

cfallin commented on PR #14418:

I think I will pass the review baton to @alexcrichton on this one if that's OK -- I'm not familiar enough with the innards of concurrent.rs to be confident in reviewing this work. Thank you though!

view this post on Zulip Wasmtime GitHub notifications bot (Sep 26 2026 at 00:19):

cfallin requested alexcrichton for a review on PR #14418.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 26 2026 at 00:19):

cfallin unassigned cfallin from PR #14418 component: keep guest fibers in their thread state.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 08:46):

Byte-Naut updated PR #14418.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 08:47):

Byte-Naut edited PR #14418:

Fixes #14241.

This supersedes #14385 and follows @alexcrichton's suggestion there: rather than guarding each handoff, keep fibers at rest in the table and only take them out while they're being resumed.

Motivation

Several scheduler paths in concurrent.rs moved a live fiber out of store-owned state (into a WaitMode, a WorkItem, or a local) and then ran a fallible step before it reached its next owner. These only fail once an internal invariant is already broken, but when they do the unfinished fiber is dropped, StoreFiber's Drop panics, and a trap or bail_bug! turns into a host panic or process abort. #14382 added two more such paths (next_switch_item in YieldingToSubtask, and the other arm in subtask_cancel).

Changes

No public API changes, no new locks, no per-suspension allocations, and no new ConcurrentState fields. About 270 of the added lines are tests.

Design

FiberKind keeps the distinctions the thread intrinsics relied on when the fiber's location implied the state:

Before Fiber kept in Now
Running, blocked in a waitable set WaitMode::Fiber FiberKind::Waiting
Running, queued as ResumeFiber or yielding to a subtask work item / next_switch_item FiberKind::Scheduled
Suspended thread state FiberKind::Suspended
Ready, queued as ResumeThread thread state FiberKind::Ready

GuestThreadState::can_resume applies the same rules as before: only Suspended (or a not-yet-started explicit thread) can be resumed, and only Ready can be promoted. Waiting and Scheduled behave like Running. resume_work_item_fiber checks that a ResumeFiber item finds Scheduled and a ResumeThread item finds Ready before taking the fiber.

Two things worth a look in review:

None of the details here are fixed. If other names for FiberKind's variants, or fewer or differently placed tests, would be easier to review, I'm happy to rework it.

Testing

thread-wait-resume.wast (new) blocks a thread in waitable-set.wait with no pending event, then calls thread.resume-later or thread.suspend-then-resume on it and expects cannot resume thread which is not suspended. It guards against a waiting thread being treated as suspended now that its fiber lives in the thread state.

The unit tests in concurrent.rs (fiber_ownership_tests) set up broken states directly: invalid wait set, thread, or subtask handles when a fiber suspends; an invalid waiter in mark_ready; an occupied switch slot; set_thread_running and cleanup_thread on a thread that still owns a fiber; and dropping a resume future before it's polled. Each checks that the original error (or bail_bug! panic in debug builds) comes back and that the fiber is still owned by the store or has been disposed of. One more test checks the resume and promote rules for each FiberKind.

cargo test -p wasmtime --lib fiber_ownership_tests
cargo test --test wast -- component-model/async

Locally: fmt, clippy for wasmtime, the full wasmtime lib tests, the async component-model .wast tests on Cranelift and Winch, the component_model tests in tests/all, the focused tests with debug assertions disabled, and no_std / non-async builds all pass.

Reviewing

The change is split into five commits that each build on their own:

  1. Keep guest fibers in their thread state. The core move: FiberKind, GuestThreadState::Fiber, id-only wait sets and work items, and thread-wait-resume.wast. This is the design choice worth checking first.
  2. Dispose of a resuming fiber that is not put back. The guard in resume_fiber, which covers only the resume interval. Most of the diff is re-indentation into the async block, so "Hide whitespace" helps.
  3. Match fiber owners exhaustively in store teardown. Follows the #14146 review suggestion.
  4. Check thread state before storing or discarding a fiber. Extra bail_bug!s for broken invariants.
  5. Tests for the broken-invariant paths.

3 and 4 are hardening on top of the fix. If you'd rather keep this smaller, I'm happy to drop them or move them to a follow-up.

Developed with assistance from Claude (Anthropic). Per the Bytecode Alliance AI Tool Use Policy, I'm the author and accountable for this change.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 21:48):

alexcrichton commented on PR #14418:

It's been a bit of a busy week for me and this is a large enough change that I want to make sure I've got sufficient time to sit down and review this, so mostly wanted to say I haven't forgotten this @Byte-Naut just taking some time to review it.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 00:57):

Byte-Naut commented on PR #14418:

It's been a bit of a busy week for me and this is a large enough change that I want to make sure I've got sufficient time to sit down and review this, so mostly wanted to say I haven't forgotten this @Byte-Naut just taking some time to review it.

Thanks for letting me know, no rush at all. Happy to adjust whatever you'd like once you get to it.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 05 2026 at 16:46):

:memo: alexcrichton submitted PR review:

Ok I've gotten a chance to read this now, thanks for your patience. Overall I'm a bit fearful of how this turned out. Whenever we add state to async things it become quite difficult to reconcile that new state space with all the preexisting state spaces and is often the source of bugs. For example adding FiberKind to the mix here seems like it's multiplying the state space further. I'm finding it personally pretty difficult to follow the refactor here to understand all of these state transitions.

My inclination of "only have fibers live in the store" might just be flat-out wrong here. One example from this PR is that the change to resume_fiber looks correct to me (along with the ResumingFiber abstraction). Otherwise though I'm fearful of the additional state being a bit too complicated to manage.

I don't know how best to resolve the original issue myself. Do you have ideas/opinions yourself?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 06 2026 at 02:09):

Byte-Naut commented on PR #14418:

Ok I've gotten a chance to read this now, thanks for your patience. Overall I'm a bit fearful of how this turned out. Whenever we add state to async things it become quite difficult to reconcile that new state space with all the preexisting state spaces and is often the source of bugs. For example adding FiberKind to the mix here seems like it's multiplying the state space further. I'm finding it personally pretty difficult to follow the refactor here to understand all of these state transitions.

My inclination of "only have fibers live in the store" might just be flat-out wrong here. One example from this PR is that the change to resume_fiber looks correct to me (along with the ResumingFiber abstraction). Otherwise though I'm fearful of the additional state being a bit too complicated to manage.

I don't know how best to resolve the original issue myself. Do you have ideas/opinions yourself?

I agree that this grew into a larger scheduler refactor than the original ownership bug warrants. FiberKind was added to preserve distinctions that the first ready flag had lost: a thread waiting on a set must still look Running to thread.resume, and a queued ResumeFiber must not behave like a Ready thread for promotion. Those distinctions already existed across the thread state, wait table, and work items. Moving them into the thread adds consistency requirements between those structures, and the current representation exposes rather than hides those requirements

I would keep the ResumingFiber protection around resume_fiber, including constructing the guard before returning the future so cancellation before the first poll is covered. I would then separate the remaining ownership fixes from the broader change to where suspended fibers live. Keeping the existing scheduler representation and concentrating the guarded take and handoff operations seems like a smaller next step. That still needs an audit of every transfer; the resume_fiber guard alone does not fix the whole issue.

If we continue with fibers resting in their threads, one option is to use direct Waiting, Scheduled, Suspended, and Ready variants instead of Fiber plus FiberKind. That removes the nested representation while preserving the checks. It does not reduce the actual scheduling states or the need to keep queue and wait records consistent. I would also be cautious about folding everything into Running with an optional fiber: it would need another reliable way to distinguish a waiter from an already scheduled thread.

My preference is to pursue the smaller ownership repair first and assess the representation change separately. The local checks support these distinctions, but I have not implemented or run the full Wasmtime suites for either proposed simplification now.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 07 2026 at 03:57):

Byte-Naut edited a comment on PR #14418:

Ok I've gotten a chance to read this now, thanks for your patience. Overall I'm a bit fearful of how this turned out. Whenever we add state to async things it become quite difficult to reconcile that new state space with all the preexisting state spaces and is often the source of bugs. For example adding FiberKind to the mix here seems like it's multiplying the state space further. I'm finding it personally pretty difficult to follow the refactor here to understand all of these state transitions.

My inclination of "only have fibers live in the store" might just be flat-out wrong here. One example from this PR is that the change to resume_fiber looks correct to me (along with the ResumingFiber abstraction). Otherwise though I'm fearful of the additional state being a bit too complicated to manage.

I don't know how best to resolve the original issue myself. Do you have ideas/opinions yourself?

I agree that this grew into a larger scheduler refactor than the original ownership bug warrants. FiberKind was added to preserve distinctions that the first ready flag had lost: a thread waiting on a set must still look Running to thread.resume, and a queued ResumeFiber must not behave like a Ready thread for promotion. Those distinctions already existed across the thread state, wait table, and work items. Moving them into the thread adds consistency requirements between those structures, and the current representation exposes rather than hides those requirements

I would keep the ResumingFiber protection around resume_fiber, including constructing the guard before returning the future so cancellation before the first poll is covered. I would then separate the remaining ownership fixes from the broader change to where suspended fibers live. Keeping the existing scheduler representation and concentrating the guarded take and handoff operations seems like a smaller next step. That still needs an audit of every transfer; the resume_fiber guard alone does not fix the whole issue.

If we continue with fibers resting in their threads, one option is to use direct Waiting, Scheduled, Suspended, and Ready variants instead of Fiber plus FiberKind. That removes the nested representation while preserving the checks. It does not reduce the actual scheduling states or the need to keep queue and wait records consistent. I would also be cautious about folding everything into Running with an optional fiber: it would need another reliable way to distinguish a waiter from an already scheduled thread.

My preference is to pursue the smaller ownership repair first and assess the representation change separately (the local checks support these distinctions, but I have not implemented or run the full Wasmtime suites for either proposed simplification now).

view this post on Zulip Wasmtime GitHub notifications bot (Oct 07 2026 at 03:58):

Byte-Naut edited a comment on PR #14418:

Ok I've gotten a chance to read this now, thanks for your patience. Overall I'm a bit fearful of how this turned out. Whenever we add state to async things it become quite difficult to reconcile that new state space with all the preexisting state spaces and is often the source of bugs. For example adding FiberKind to the mix here seems like it's multiplying the state space further. I'm finding it personally pretty difficult to follow the refactor here to understand all of these state transitions.

My inclination of "only have fibers live in the store" might just be flat-out wrong here. One example from this PR is that the change to resume_fiber looks correct to me (along with the ResumingFiber abstraction). Otherwise though I'm fearful of the additional state being a bit too complicated to manage.

I don't know how best to resolve the original issue myself. Do you have ideas/opinions yourself?

Thanks for reviewing. I agree that this grew into a larger scheduler refactor than the original ownership bug warrants. FiberKind was added to preserve distinctions that the first ready flag had lost: a thread waiting on a set must still look Running to thread.resume, and a queued ResumeFiber must not behave like a Ready thread for promotion. Those distinctions already existed across the thread state, wait table, and work items. Moving them into the thread adds consistency requirements between those structures, and the current representation exposes rather than hides those requirements

I would keep the ResumingFiber protection around resume_fiber, including constructing the guard before returning the future so cancellation before the first poll is covered. I would then separate the remaining ownership fixes from the broader change to where suspended fibers live. Keeping the existing scheduler representation and concentrating the guarded take and handoff operations seems like a smaller next step. That still needs an audit of every transfer; the resume_fiber guard alone does not fix the whole issue.

If we continue with fibers resting in their threads, one option is to use direct Waiting, Scheduled, Suspended, and Ready variants instead of Fiber plus FiberKind. That removes the nested representation while preserving the checks. It does not reduce the actual scheduling states or the need to keep queue and wait records consistent. I would also be cautious about folding everything into Running with an optional fiber: it would need another reliable way to distinguish a waiter from an already scheduled thread.

My preference is to pursue the smaller ownership repair first and assess the representation change separately (the local checks support these distinctions, but I have not implemented or run the full Wasmtime suites for either proposed simplification now).

view this post on Zulip Wasmtime GitHub notifications bot (Oct 07 2026 at 22:10):

alexcrichton commented on PR #14418:

I'd personally be amenable to seeing how things looked, but I don't have a great vision in my head of what you're describing insofar as I don't feel "yeah for sure we want that" or "no I don't think that'll work". If you're up to experiment though it'd be appreciated!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 03:04):

:cross_mark: Byte-Naut closed without merge PR #14418.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 03:04):

Byte-Naut commented on PR #14418:

I'd personally be amenable to seeing how things looked, but I don't have a great vision in my head of what you're describing insofar as I don't feel "yeah for sure we want that" or "no I don't think that'll work". If you're up to experiment though it'd be appreciated!

Thanks for the guidance, happy to help sort this out. I've put together an implementation of the smaller path and opened it as #14620. The scheduler representation is unchanged — WaitMode::Fiber and WorkItem::ResumeFiber still carry the fiber, and the fix adds a ResumingFiber guard that covers the resume interval from the take to the handoff, plus moving the fallible checks before each ownership transfer.

This is based on my own reading of the code, so I'd welcome any correction on the overall approach or on where the ownership should live. Happy to adjust, do more testing, or answer questions whenever you have time.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 03:05):

Byte-Naut reopened PR #14418 from Byte-Naut:issue-14241-2 to bytecodealliance:main.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 03:07):

Byte-Naut edited a comment on PR #14418:

I'd personally be amenable to seeing how things looked, but I don't have a great vision in my head of what you're describing insofar as I don't feel "yeah for sure we want that" or "no I don't think that'll work". If you're up to experiment though it'd be appreciated!

Thanks for the guidance, happy to help sort this out. I've put together an implementation of the smaller path and opened it as #14620. The scheduler representation is unchanged — WaitMode::Fiber and WorkItem::ResumeFiber still carry the fiber, and the fix adds a ResumingFiber guard that covers the resume interval from the take to the handoff, plus moving the fallible checks before each ownership transfer.

This is based on my own reading of the code, so I'd welcome any correction on the overall approach or on where the ownership should live. Glad to adjust, do more testing, or answer questions whenever you have time.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 03:18):

Byte-Naut edited a comment on PR #14418:

I'd personally be amenable to seeing how things looked, but I don't have a great vision in my head of what you're describing insofar as I don't feel "yeah for sure we want that" or "no I don't think that'll work". If you're up to experiment though it'd be appreciated!

Thanks for the guidance, happy to help sort this out. I've put together an implementation of the smaller path and opened it as #14620. The scheduler representation is unchanged — WaitMode::Fiber and WorkItem::ResumeFiber still carry the fiber, and the fix adds a ResumingFiber guard and moves the fallible checks before each ownership transfer.

Happy to hear if the representation or the scope looks off — and glad to adjust, do more testing, or answer questions whenever you have time.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 03:19):

Byte-Naut edited a comment on PR #14418:

I'd personally be amenable to seeing how things looked, but I don't have a great vision in my head of what you're describing insofar as I don't feel "yeah for sure we want that" or "no I don't think that'll work". If you're up to experiment though it'd be appreciated!

Thanks for the guidance, happy to help sort this out. I've put together an implementation of the smaller path and opened it as #14620. The scheduler representation is unchanged — WaitMode::Fiber and WorkItem::ResumeFiber still carry the fiber, and the fix adds a ResumingFiber guard and moves the fallible checks before each ownership transfer.

Happy to hear if the representation or the scope looks off — and glad to adjust, do more testing, or answer questions whenever you have time!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 03:19):

Byte-Naut edited a comment on PR #14418:

I'd personally be amenable to seeing how things looked, but I don't have a great vision in my head of what you're describing insofar as I don't feel "yeah for sure we want that" or "no I don't think that'll work". If you're up to experiment though it'd be appreciated!

Thanks for the guidance, happy to help sort this out. I've put together an implementation of the smaller path and opened it as #14620. The scheduler representation is unchanged — WaitMode::Fiber and WorkItem::ResumeFiber still carry the fiber, and the fix adds a ResumingFiber guard and moves the fallible checks before each ownership transfer.

Happy to hear if the representation or the scope looks off — and glad to adjust, do more testing, or answer questions whenever you have time.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 10 2026 at 06:33):

:cross_mark: Byte-Naut closed without merge PR #14418.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 10 2026 at 06:33):

Byte-Naut commented on PR #14418:

Closing in favor of #14620.


Last updated: Oct 11 2026 at 04:10 UTC