Stream: git-wasmtime

Topic: wasmtime / PR #11353 ensure component value lowerings are...

Wasmtime GitHub notifications bot (Jul 30 2025 at 19:54):

dicej requested alexcrichton for a review on PR #11353.

Wasmtime GitHub notifications bot (Jul 30 2025 at 19:54):

dicej requested wasmtime-core-reviewers for a review on PR #11353.

Wasmtime GitHub notifications bot (Jul 30 2025 at 19:54):

dicej opened PR #11353 from dicej:lower-on-worker-fiber to bytecodealliance:main:

This is necessary because lowering a component value may require calling realloc, which may involve epoch interruption, which may require suspending the current fiber. Which obviously doesn't work if we're not running in a fiber. Also, we need to make sure there are no host frames on the stack.

Note the use of Mutex for WorkItem::WorkerFunction and WorkerItem::Function. We never lock the mutex -- it's only used to plumb through the inner, non-Sync value while satisfying the compiler that Store is Sync.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Jul 30 2025 at 19:58):

dicej commented on PR #11353:

This should fix https://github.com/bytecodealliance/wasmtime/issues/11348

Wasmtime GitHub notifications bot (Jul 30 2025 at 19:59):

dicej edited a comment on PR #11353:

This should fix https://github.com/bytecodealliance/wasmtime/issues/11348 (and I've verified tests pass with Alex's assertion).

Wasmtime GitHub notifications bot (Jul 30 2025 at 20:04):

alexcrichton submitted PR review:

Nice! That was easier to fix than expected. Happy to see this merged whenever, but a few thoughts too:

• For Mutex I've been wondering if we should have a wrapper of some kind that sort of looks like a Mutex but is just a newtype wrapper. Basically it would disallow access to the underlying data with &self and would exclusively provide access via &mut self. That would support unsafe impl<T: Send> Sync for TheWrapper<T>. There's another example of this in the codebase here too. The only benefit of this would be avoiding the cost of creating the mutex, which I don't think is too costly nowadays, so no need to get to this any time soon (but would be good to keep in our back pocket I think).
• Could the assertion be added in this PR too? I'm imagining something like where do leave it in LowerContext::new and then for the "fast path" case we provide an alternate constructor which is something like LowerContext::new_assert_no_realloc which would panic if realloc is called (and avoid the debug assertion). Or something like that, basically still have the debug assertion as much as we can but provide a hatch for the fast-path we're thinking of adding.

Wasmtime GitHub notifications bot (Jul 30 2025 at 20:16):

dicej updated PR #11353.

Wasmtime GitHub notifications bot (Jul 30 2025 at 20:17):

dicej commented on PR #11353:

I just added the assertion. I had to gate it on the component-model-async feature because StoreOpaque::with_blocking is gated on that. Alternatively, we could ungate them both.

Wasmtime GitHub notifications bot (Jul 30 2025 at 20:19):

dicej commented on PR #11353:

Agreed about using something simpler than a Mutex to make stuff like this Sync -- performance aside, it's confusing to readers to have Mutexs all over the place that are never locked.

Wasmtime GitHub notifications bot (Jul 30 2025 at 20:20):

alexcrichton commented on PR #11353:

It's ok to leave it gated for now, eventually I think we're going to want to lift more out of the feature gate but it's ok to defer that to later.

Wasmtime GitHub notifications bot (Jul 30 2025 at 21:17):

dicej merged PR #11353.

Last updated: Dec 06 2025 at 06:05 UTC