Stream: git-wasmtime

Topic: wasmtime / PR #14385 component: keep fibers owned by the ...

Wasmtime GitHub notifications bot (Sep 23 2026 at 10:35):

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

Part of #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 handoffs I found 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.

Wasmtime GitHub notifications bot (Sep 23 2026 at 10:35):

Byte-Naut requested pchickey for a review on PR #14385.

Wasmtime GitHub notifications bot (Sep 23 2026 at 10:35):

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

Wasmtime GitHub notifications bot (Sep 23 2026 at 10:49):

Byte-Naut updated PR #14385.

Wasmtime GitHub notifications bot (Sep 23 2026 at 10:49):

Byte-Naut edited PR #14385:

Fixes #14241.

Motivation

While reviewing #14146, @alexcrichton [pointed out](https://github.com/bytecodealliance/wasmtime/pull/14146#discussion_r3883919087) 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.

Wasmtime GitHub notifications bot (Sep 23 2026 at 10:59):

Byte-Naut edited PR #14385:

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.

Wasmtime GitHub notifications bot (Sep 23 2026 at 12:53):

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

Wasmtime GitHub notifications bot (Sep 23 2026 at 13:43):

dicej requested dicej for a review on PR #14385.

Wasmtime GitHub notifications bot (Sep 23 2026 at 14:02):

dicej commented on PR #14385:

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.

Wasmtime GitHub notifications bot (Sep 23 2026 at 14:14):

:memo: dicej submitted PR review:

LGTM, thanks; just a couple of suggestions inline.

Regarding the conflicts with https://github.com/bytecodealliance/wasmtime/pull/14382: let me know if you need help resolving them.

Wasmtime GitHub notifications bot (Sep 23 2026 at 14:14):

:speech_balloon: dicej created PR review comment:

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

Wasmtime GitHub notifications bot (Sep 23 2026 at 14:14):

:speech_balloon: dicej created PR review comment:

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

Wasmtime GitHub notifications bot (Sep 23 2026 at 15:20):

alexcrichton commented on PR #14385:

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.

Wasmtime GitHub notifications bot (Sep 24 2026 at 00:01):

Byte-Naut commented on PR #14385:

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.

Wasmtime GitHub notifications bot (Sep 25 2026 at 06:11):

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

Wasmtime GitHub notifications bot (Sep 25 2026 at 06:11):

Byte-Naut commented on PR #14385:

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.

Last updated: Oct 11 2026 at 04:10 UTC