Stream: git-wasmtime

Topic: wasmtime / PR #12441 Add some more OOM-handling `Box<[T]>...

Wasmtime GitHub notifications bot (Jan 26 2026 at 22:59):

fitzgen opened PR #12441 from fitzgen:more-oom-handling-boxed-slice-ctors to bytecodealliance:main:

Part of https://github.com/bytecodealliance/wasmtime/issues/12069

<!--
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 (Jan 26 2026 at 22:59):

fitzgen requested cfallin for a review on PR #12441.

Wasmtime GitHub notifications bot (Jan 26 2026 at 22:59):

fitzgen requested wasmtime-compiler-reviewers for a review on PR #12441.

Wasmtime GitHub notifications bot (Jan 26 2026 at 22:59):

fitzgen requested pchickey for a review on PR #12441.

Wasmtime GitHub notifications bot (Jan 26 2026 at 22:59):

fitzgen requested wasmtime-core-reviewers for a review on PR #12441.

Wasmtime GitHub notifications bot (Jan 26 2026 at 23:01):

fitzgen updated PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 01:05):

github-actions[bot] added the label cranelift on PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:06):

alexcrichton submitted PR review:

Another possible idea:

• One function to return fn(usize) -> Result<Box<[MaybeUninit<T>]>>
• Another function fn(Iterator<T>, Box<[MaybeUninit<T>]>) -> Result<Box<[T]>>
• Another function for a fallible iterator
• Helper functions combining these together

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:06):

alexcrichton created PR review comment:

Once this goes down the route of double/shrink/realloc, would it be better to use Vec directly?

Or, alternatively, could the Result-based fallible-collection-version take an ExactSizeIterator to avoid resizing?

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:18):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:18):

fitzgen created PR review comment:

Once this goes down the route of double/shrink/realloc, would it be better to use Vec directly?

I considered this, but we would still need some raw pointer handling for the shrink_to_fit operation, since there isn't a fallible way to do that with std::vec::Vec. And deconstructing and reconstructing a std::vec::Vec felt even more sketchy than what we have here.

Or, alternatively, could the Result-based fallible-collection-version take an ExactSizeIterator to avoid resizing?

Unfortunately we have code that needs to call these methods but which doesn't have an ExactSizeIterator, so we can't replace all these methods with that. Basically, we really do need all the different variants in different places.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:22):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:22):

alexcrichton created PR review comment:

To me I would naively weigh this implementation as "a greater evil" than assuming shrink_to_fit is infallible. I understand that shrink_to_fit is technically fallible but it's hard to imagine how any realistic allocator would do anything other than succeed at shrinking the allocation. I think I'd almost prefer to make that gamble than to half-build our own custom Vec internally.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:26):

fitzgen commented on PR #12441:

Another possible idea:

* One function to return `fn(usize) -> Result<Box<[MaybeUninit<T>]>>`

* Another function `fn(Iterator<T>, Box<[MaybeUninit<T>]>) -> Result<Box<[T]>>`

* Another function for a fallible iterator

* Helper functions combining these together

This is sort of what we have, except we don't expose the second function publicly, just as an implementation detail.

What these proposed functions don't collectively cover, however, is when you have an unknown-length iterator, unless I am misunderstanding.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:26):

fitzgen edited a comment on PR #12441:

Another possible idea:

• One function to return fn(usize) -> Result<Box<[MaybeUninit<T>]>>

• Another function fn(Iterator<T>, Box<[MaybeUninit<T>]>) -> Result<Box<[T]>>

• Another function for a fallible iterator

• Helper functions combining these together

This is sort of what we have, except we don't expose the second function publicly, just as an implementation detail.

What these proposed functions don't collectively cover, however, is when you have an unknown-length iterator, unless I am misunderstanding.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:29):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:29):

fitzgen created PR review comment:

I understand that shrink_to_fit is technically fallible but it's hard to imagine how any realistic allocator would do anything other than succeed at shrinking the allocation.

If the allocation can't be shrunk in place (e.g. due to size classes) then it must allocate a new block of memory and copy the contents of the old block over. That allocation, despite being smaller than the existing one, can certainly fail.

I think I'd almost prefer to make that gamble than to half-build our own custom Vec internally.

I can refactor this to build on top of https://github.com/bytecodealliance/wasmtime/pull/12440 and do some unsafe bits to fallibly shrink in place.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:29):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:29):

alexcrichton created PR review comment:

Also, technically, like Box we can convert a Vec<T> to its raw parts and call realloc manually to handle the edge case it returns null? (and return oom)

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:31):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:31):

fitzgen created PR review comment:

Also, technically, like Box we can convert a Vec<T> to its raw parts and call realloc manually to handle the edge case it returns null? (and return oom)

Yeah this is the "unsafe bits to fallibly shrink in place" stuff

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:31):

fitzgen edited PR review comment.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:32):

fitzgen edited PR review comment.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:38):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 27 2026 at 02:38):

alexcrichton created PR review comment:

Oh sorry yeah I think we raced to the same realization, but yeah building this on Vec with a custom hand-written shrink_to_fit sounds good to me :+1:

Wasmtime GitHub notifications bot (Jan 27 2026 at 18:09):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 27 2026 at 18:09):

fitzgen created PR review comment:

I haven't done this change yet, but it will require https://github.com/bytecodealliance/wasmtime/pull/12448 to land first.

Wasmtime GitHub notifications bot (Jan 27 2026 at 19:13):

fitzgen updated PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 19:14):

fitzgen commented on PR #12441:

Okay the boxed slice helpers are now implemented on top of our OOM-handling Vec and this PR now depends on https://github.com/bytecodealliance/wasmtime/pull/12452 (and therefore also https://github.com/bytecodealliance/wasmtime/pull/12448)

Wasmtime GitHub notifications bot (Jan 27 2026 at 19:14):

fitzgen requested alexcrichton for a review on PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 20:22):

fitzgen updated PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 22:08):

cfallin submitted PR review:

This looks correct to me; I think my main question (in addition to the one naming thought below) is why we need to define an error case TooFewItems. I suppose I'm not seeing the callsite(s) here but naively I would expect, given that we've implemented the shrink_to_fit logic, we could always use that if an iterator returns fewer items than expected; do we have cases where we want to guard against that (reallocation) wastefulness and flag a logic error instead?

Wasmtime GitHub notifications bot (Jan 27 2026 at 22:08):

cfallin created PR review comment:

The name DropGuard is throwing me off a bit -- yes it exists to drop the Vec<T> but usually (in my experience) a drop-guard is a thing that just handles drops (whether via explicit/custom impl Drop or not), nothing else, while the drop here is incidental (it owns the Vec so everything cleans up as expected) and the real meat is in the other logic. The "cleans up already-initialized elements" suggests something like a MaybeUninit somewhere but that's not the case either. Maybe BoxedSliceBuilder or something?

Wasmtime GitHub notifications bot (Jan 27 2026 at 22:16):

fitzgen commented on PR #12441:

do we have cases where we want to guard against that (reallocation) wastefulness and flag a logic error instead?

Basically, yes. We have call sites that know the exact length of various things but which also do not have ExactSizeIterators for Reasons.

Wasmtime GitHub notifications bot (Jan 27 2026 at 22:37):

fitzgen updated PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 22:37):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 27 2026 at 22:37):

fitzgen created PR review comment:

Done in f9076a506c

Wasmtime GitHub notifications bot (Jan 27 2026 at 22:37):

fitzgen has enabled auto merge for PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 23:03):

fitzgen updated PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 23:24):

fitzgen updated PR #12441.

Wasmtime GitHub notifications bot (Jan 27 2026 at 23:37):

fitzgen added PR #12441 Add some more OOM-handling Box<[T]> constructor variants to the merge queue.

Wasmtime GitHub notifications bot (Jan 27 2026 at 23:59):

github-merge-queue[bot] removed PR #12441 Add some more OOM-handling Box<[T]> constructor variants from the merge queue.

Wasmtime GitHub notifications bot (Jan 28 2026 at 01:04):

fitzgen added PR #12441 Add some more OOM-handling Box<[T]> constructor variants to the merge queue.

Wasmtime GitHub notifications bot (Jan 28 2026 at 01:28):

fitzgen merged PR #12441.

Wasmtime GitHub notifications bot (Jan 28 2026 at 01:28):

fitzgen removed PR #12441 Add some more OOM-handling Box<[T]> constructor variants from the merge queue.

Last updated: Jan 29 2026 at 13:25 UTC