Stream: git-wasmtime

Topic: wasmtime / PR #10943 Crack down on mutability and ownersh...

Wasmtime GitHub notifications bot (Jun 05 2025 at 22:57):

alexcrichton opened PR #10943 from alexcrichton:instance-pin to bytecodealliance:main:

This commit represents more effort to bring safety to vm::Instance and, eventually, ComponentInstance. This is specifically addressing two points of safety around vm::Instance:

• Previously ownership of this was murky where InstanceHandle sort of represented ownership but sort of didn't either through the InstanceHandle::clone method. Now InstanceHandle has a destructor for instances and no longer has clone, so there's one exclusive owner of an instance.

• Previously &mut Instance was liberally passed around, but this is not sound because certain fields cannot be mutated (e.g. runtime offset information). While not a perfect solution this PR switches to using Pin<&mut Instance> everywhere instead. This prevents safe access to &mut Instance and we hand-write accessors to individual fields. Notably we omit mutable access to the runtime_info field.

This naturally involved a lot of refactoring internally, but notably this started bringing up preexisting issues around how there are locations in the codebase that simultaneously have &mut Instance and &mut StoreOpaque which is technically not sound due to being able to get back to the instance from the store. Some issues here were address by passing around indices more often such as in instance initialization and const-expr evaluation.

Note that all proxy methods on InstanceHandle are also all removed now and there's now only two: get and get_mut. This reflects how InstanceHandle should in general no longer be used and instead Instance itself, and some pointer-to thereof, should be exclusively used.

cc #10933

<!--
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 (Jun 05 2025 at 22:57):

alexcrichton requested fitzgen for a review on PR #10943.

Wasmtime GitHub notifications bot (Jun 05 2025 at 22:57):

alexcrichton requested wasmtime-core-reviewers for a review on PR #10943.

Wasmtime GitHub notifications bot (Jun 05 2025 at 23:00):

alexcrichton updated PR #10943.

Wasmtime GitHub notifications bot (Jun 05 2025 at 23:09):

alexcrichton updated PR #10943.

Wasmtime GitHub notifications bot (Jun 05 2025 at 23:10):

fitzgen submitted PR review:

nice

Wasmtime GitHub notifications bot (Jun 05 2025 at 23:12):

alexcrichton commented on PR #10943:

I'll also note that ownership-wise this brings core wasm in line with components, where components have OwnedComponentInstance which is now basically the exact same thing as InstanceHandle (and I'll try to eventually get around to aligning on naming conventions too)

Wasmtime GitHub notifications bot (Jun 05 2025 at 23:17):

alexcrichton has enabled auto merge for PR #10943.

Wasmtime GitHub notifications bot (Jun 05 2025 at 23:58):

alexcrichton commented on PR #10943:

While I have no reason to believe that's a flaky test failure I'm neverthless curious. If it's flaky that's quite worrisome. If it's not flaky that's also worrisome...

Wasmtime GitHub notifications bot (Jun 06 2025 at 00:24):

alexcrichton commented on PR #10943:

Ok spurious failures should be fixed by https://github.com/bytecodealliance/wasmtime/pull/10947

Wasmtime GitHub notifications bot (Jun 06 2025 at 16:41):

alexcrichton merged PR #10943.

Last updated: Dec 06 2025 at 07:03 UTC