zzjas opened PR #12482 from zzjas:issue-12456-drc-leak to bytecodealliance:main:
When
StructRef::neworArrayRef::new_fixedfails midway,dealloc_uninitfrees the object without dec-refing GC refs already written to earlier fields, leaking them. Fix by zero-filling the object body inalloc_rawso trace_gc_ref skips uninitialized slots, then tracing and dec-refing children indealloc_uninitbefore freeing.Fixes #12456
<!--
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.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
zzjas requested wasmtime-core-reviewers for a review on PR #12482.
zzjas requested alexcrichton for a review on PR #12482.
cc: @fitzgen
github-actions[bot] added the label wasmtime:api on PR #12482.
github-actions[bot] added the label wasmtime:ref-types on PR #12482.
github-actions[bot] commented on PR #12482:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"Thus the following users have been cc'd because of the following labels:
- fitzgen: wasmtime:ref-types
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
alexcrichton submitted PR review:
Thanks for this!
Thinking about this a bit more though, it feels like it's a lot of infrastructure here for not a ton of benefit. For example this pessimizes all allocations to be zero-filled instead of truly uninitialized. This isn't even used by compiled guest wasm and it's also only used on the host, a secondary vector for implementing GC things.
Given all that, I'm wondering if we should step back a bit and reconsider the basic strategy here. For example I would naively expect that one way to do this would be to enhance the type-check we have for initializing structs/arrays/etc on the host such that the initialization step never fails. We already do a type-check there and it looks like the externref bits aren't fully checked there. One other way to tackle this problem would be to (1) enhance the type-check to avoid failing during initialization and (2) use
.unwrap()to assert that per-field initialization succeeds.cc @fitzgen on this as well, you likely have thoughts too! I also think there's a case to be made for "let's fix the bug first and then make it fast later", in which case I think this PR is perfectly suitable. The only downside is that it puts things in a bit of a weird state where the "uninit" terminology is now more of a "maybe uninit maybe not" which is a bit confusing. Not enough to block landing this though, but that's what led me to reconsider the overall approach.
fitzgen commented on PR #12482:
I like the idea of checking that references are properly rooted at type-checking time, and
unwraping during field initialization.My other suggestion would have been to keep track of how many fields have been initialized and then drop the initialized fields in an
Undo, but I like checking ahead of time better.
Last updated: Feb 24 2026 at 04:36 UTC