component: keep fibers owned by the store on failed handoffs

[Byte-Naut]

Fixes #14241.

Motivation

While reviewing #14146, @alexcrichton pointed out that many places in concurrent.rs have a live fiber in scope across a ?. These only fail when an internal invariant is already broken, but if one did, the unfinished fiber would be dropped and StoreFiber's Drop would panic, turning a trap or bail_bug! into a process abort. @dicej filed #14241 to track this and described the two safe options: keep the fiber owned by the store, or hold it in an RAII guard that disposes of it through the store.

This PR applies those two options to the fiber handoffs in concurrent.rs.

Changes

• resume_fiber and run_on_worker: hold the fiber in a small private DisposeFiber guard until it reaches its destination, so an early return disposes of it through the existing StoreFiber::dispose. In the Yielding and ExplicitlySuspending arms the thread lookup now happens before the fiber is taken; previously the right-hand side of the assignment took the fiber first. The two waiter arms use Entry so the fiber only moves into a vacant slot.
• Waitable::mark_ready: do the fallible lookups while the waiter is still in its set, and only remove it once its destination has been checked.
• set_switch_item: if the slot is already occupied, push the incoming item onto the high-priority queue before the existing bail_bug!, so the store still owns its fiber.
• handle_work_item (ResumeThread) and subtask_cancel: put the fiber-owning state back before the existing bail_bug!s.
• resume_thread: reuse the already-borrowed ConcurrentState instead of looking it up again after taking the fiber.

Not changed: happy-path scheduling, error values and messages, the panic for duplicate waiters, and the assertion in StoreFiber::drop for fibers that were never disposed. There are no public API changes. About 200 of the added lines are tests.

Design

DisposeFiber follows the same pattern as the existing Dispose guard in poll_until and the Drop impl of FiberFuture: it calls StoreFiber::dispose only if the fiber has not been handed off yet. Elsewhere the fix is a reordering so that the fiber is taken last, which avoids adding a guard where the lookup can simply happen first.

One thing worth calling out for review: when set_switch_item finds the slot occupied, the incoming item goes to high_priority only so that it is disposed of with the store. The bail_bug! poisons the store, so the item never runs. Happy to keep it somewhere else if that reads better.

Testing

These paths are only reachable once an invariant is already broken, so the new unit tests in concurrent.rs (fiber_disposal_tests) set up the broken state directly: an invalid waitable set, thread, or caller when a fiber suspends, an invalid waiting thread in mark_ready, and an occupied switch slot. Each test checks that the original error is returned and that the fiber is either disposed of or still owned by the store. All six abort the test process on current main and pass with this change, with and without debug assertions.

cargo test -p wasmtime --lib fiber_disposal_tests

Locally: fmt, clippy, the full wasmtime lib test suite, and the async component-model .wast tests on Cranelift and Winch pass.

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

[dicej]

Thanks, @Byte-Naut!

One thing worth calling out for review: when set_switch_item finds the slot occupied, the incoming item goes to high_priority only so that it is disposed of with the store. The bail_bug! poisons the store, so the item never runs. Happy to keep it somewhere else if that reads better.

FWIW, my solution in #14382 was to store the item in ConcurrentState::table using push and then update take_fibers_and_futures to make sure we look for it there. That's arguably a bit less confusing than using high_priority given that we have no intention of running it. I agree that, since we're poisoning the store anyway, there's no functional difference.

[dicej]

LGTM, thanks; just a couple of suggestions inline.

Regarding the conflicts with #14382: let me know if you need help resolving them.

[dicej]

A comment here to remind the reader why we're putting the object back in the store would be helpful.

[dicej]

Again, a comment here would be helpful to remind the reader why we're putting this back in the store.

[alexcrichton]

Personally I'm a bit wary of a strategy like this because everywhere a fiber might be used we still have to proactively reach for the DisposeFiber abstraction and use it. I was toying with something locally awhile back akin to what @dicej said by leaving fibers in the table and only pulling them out temporarily, but I never got around to cleaning it up (and would be happy to not have to do so). I'd personally perfer to push on that path first because by construction fibers are largely at-rest in the table and then they're only temporarily removed in the brief and readily-auditable moment where they're actually resumed.

[Byte-Naut]

Thanks both. Happy to take the "fibers stay in the table" approach instead. My rough plan: keep each fiber in its GuestThread state and have queued work items and waiters refer to the thread, taking the fiber out only while it's being resumed. That shouldn't add allocations or state on the normal path beyond a table lookup at resume time; if something does, I'll call it out. @alexcrichton does that match what you had in mind, and is there anything from your local attempt I should follow? I can do this on this PR or as a new one, whichever you prefer.

[Byte-Naut]

I've put the table-based approach up as #14418, following @alexcrichton's suggestion above: guest fibers now stay in their thread's state (or the worker slot), and are only taken out while resume_fiber runs them, so work items and waiters just carry thread ids. It also covers the two new cases from #14382, and the spots @dicej's inline comments were on no longer move fibers around, so I'll close this in favor of it.

Thanks both for the quick and clear feedback, and @dicej for the pointer on keeping items in the table.