wasmtime / PR #13918 Fix current-thread leak on concurren... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #13918 Fix current-thread leak on concurren...

Wasmtime GitHub notifications bot (Jul 21 2026 at 02:07):

SebTardif opened PR #13918 from SebTardif:fix/restore-thread-on-lower-error to bytecodealliance:main:

Summary

Temporary set_thread switches used during concurrent component lowering (so guest realloc / callbacks see the correct thread) were not restored on error paths. An early ? / bail! after set_thread left the store's current thread pointing at the temporary thread, so later concurrent operations could run with the wrong thread context.

This adds with_store_thread / restore_thread_after that always restore the previous thread (preferring the original work error if restore also fails), and converts every temporary save/restore pair in this class.

Why this is needed

#12736 correctly established that stream/future lowering can call guest realloc and therefore must set the current thread first. That PR also added restore on the success path. Error paths that return before the matching restore still leave the wrong thread installed.

Concrete example in futures_and_streams::lower when T::MAY_REQUIRE_REALLOC is true:

set_thread(caller_thread) installs the caller.

Alignment / bounds checks or linear_store_list_to_memory can fail with ?.

The matching set_thread(old_thread) never runs.

The same pattern existed for guest↔guest future/stream copy, event delivery callbacks, stackless/stackful guest calls, async return adapters, host-task result lowering, and explicit thread start.

Fix

StoreOpaque::restore_thread_after always restores after a work Result.

Free function with_store_thread for the common scoped form.

AsStoreOpaque for StoreContextMut so the helper works at those call sites.

Converted all temporary set_thread pairs found by grepping crates/wasmtime/src/runtime/component.

Permanent thread transitions (enter/exit sync call, host task create, fiber resume bookkeeping) are unchanged.

Testing

Unit tests in with_store_thread_tests:

Restores the previous current thread on success.

Restores the previous current thread when work returns an error (red-green: skipping restore fails the assertion with got None after installing a host marker thread).

Prefer work errors over restore errors.
cargo test -p wasmtime --features component-model-async --lib with_store_thread
# 3 passed
Also ran other concurrent unit tests in the crate (cargo test -p wasmtime --features component-model-async --lib concurrent).

Related

Introduced with incomplete restore coverage in #12736 (merged 2026-03-11). Review discussion there already asked about restoring the previous thread after the temporary switch.

Related current-thread API work: #13887, #13759.

Wasmtime GitHub notifications bot (Jul 21 2026 at 02:07):

SebTardif requested pchickey for a review on PR #13918.

Wasmtime GitHub notifications bot (Jul 21 2026 at 02:07):

SebTardif requested wasmtime-core-reviewers for a review on PR #13918.

Wasmtime GitHub notifications bot (Jul 21 2026 at 03:52):

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

Wasmtime GitHub notifications bot (Jul 21 2026 at 17:28):

pchickey unassigned pchickey from PR #13918 Fix current-thread leak on concurrent lower error paths.

Wasmtime GitHub notifications bot (Jul 21 2026 at 17:28):

pchickey requested alexcrichton for a review on PR #13918.

Wasmtime GitHub notifications bot (Jul 21 2026 at 19:03):

:cross_mark: alexcrichton closed without merge PR #13918.

Wasmtime GitHub notifications bot (Jul 21 2026 at 19:03):

alexcrichton commented on PR #13918:

I'd advise against blindly following LLM advice when it comes to the deep internals of the runtime, especially an async runtime. It's intentional that error paths right now do not have the same degree of cleanup as successful paths because errors indicate traps after which a component is entirely locked down and unable to be used again.

Are you able to write tests that misbehave using just wasm? Tests for the internal state aren't too interesting since they're not guaranteed to be exposed by the public API.

Wasmtime GitHub notifications bot (Jul 21 2026 at 19:03):

alexcrichton reopened PR #13918 from SebTardif:fix/restore-thread-on-lower-error to bytecodealliance:main.

Wasmtime GitHub notifications bot (Jul 21 2026 at 19:03):

alexcrichton commented on PR #13918:

er, sorry, didn't mean to close

Wasmtime GitHub notifications bot (Jul 21 2026 at 20:24):

SebTardif commented on PR #13918:

@alexcrichton

It's intentional that error paths right now do not have the same degree of cleanup as successful paths because errors indicate traps after which a component is entirely locked down and unable to be used again.

Thanks for the context. That tracks: these host-side errors (alignment/bounds in lower, callback failures) propagate via ? through poll_until, so the event loop exits before anything reads the stale thread state.

Are you able to write tests that misbehave using just wasm?

No. The event loop terminates on error before anything could observe the stale state.

Closing this; the cleanup asymmetry is consistent with the design.

Wasmtime GitHub notifications bot (Jul 21 2026 at 20:24):

:cross_mark: SebTardif closed without merge PR #13918.

Last updated: Jul 29 2026 at 05:03 UTC