component: keep guest fibers in their thread state

[Byte-Naut]

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

• Every resting guest fiber now lives in GuestThreadState::Fiber { fiber, kind }, and the reusable worker fiber stays in ConcurrentState::worker. WaitMode::Fiber and WorkItem::ResumeFiber no longer hold a fiber, so wait sets, work queues, switch_item, next_switch_item, and the saved switch items in the table only carry thread ids.
• A fiber is taken out of its thread only in handle_work_item, after checking the expected kind, and handed straight to resume_fiber. resume_fiber holds it together with the store in a small private guard until it is back in its thread or the worker slot, so a failed lookup, or dropping the future before it is polled, disposes of it through the store.
• take_fibers_and_futures and trace_fiber_roots destructure ConcurrentState and match GuestThreadState exhaustively, as suggested in the align multithreading and trap behavior with CM spec #14146 review, so a new field or variant has to be handled there. Neither needs to look in wait sets or work items for fibers anymore.
• State transitions that would overwrite or delete a thread state still holding a fiber now bail_bug! instead. restore_next_switch_item does the same if next_switch_item was set again while the saved one was stashed, since overwriting it would silently lose that switch.

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:

• set_thread_running leaves a Scheduled or Waiting fiber in place. Before, that write set a thread whose fiber lived elsewhere to Running; now the fiber stays in the thread state, which already means "running" for the intrinsics.
• The guard in resume_fiber is the only place a guest fiber is outside the table, and it covers exactly the resume interval.

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 align multithreading and trap behavior with CM spec #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.

[cfallin]

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!

[alexcrichton]

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.

[Byte-Naut]

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.

[alexcrichton]

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?

[Byte-Naut]

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).

[alexcrichton]

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!

[Byte-Naut]

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.

[Byte-Naut]

Closing in favor of #14620.