| 509df0df | 25-Aug-2025 |
Alex Crichton <[email protected]> |
Refactor resetting memory on `MemoryImageSlot` drop (#11510)
* Refactor resetting memory on `MemoryImageSlot` drop
This commit refactors the behavior of dropping a `MemoryImageSlot` to no
longer ma
Refactor resetting memory on `MemoryImageSlot` drop (#11510)
* Refactor resetting memory on `MemoryImageSlot` drop
This commit refactors the behavior of dropping a `MemoryImageSlot` to no
longer map anonymous memory into the slot. This behavior was implemented
previously because if a `MemoryImageSlot` is dropped then the state of
the slot is unknown and to prevent any sort of data leakage a reset is
performed.
This reset operation, however, is fallible in that it calls `mmap`.
Calls to `mmap` can fail due to `ENOMEM`, for example, if the process
has reached its VMA limit. This means that if a process is in a near-OOM
condition then failing to allocate a memory image could panic the
process due to the `unwrap()` in the destructor of `MemoryImageSlot`.
The purpose of this commit is to avoid this `unwrap()` and instead move
the reset behavior to a location where an error can be propagated.
This commit removes the clear-on-drop behavior of `MemoryImageSlot`
slot. This was already disabled everywhere except the pooling allocator.
The pooling allocator now maintains an extra bit of state-per-slot where
instead of storing `Option<MemoryImageSlot>` it now stores effectively
one other variant of "unknown". On reuse of an "unknown" slot the memory
is reset back to an anonymous mapping and this is all done in a context
where an error can be propagated.
Two tests are added in this commit to confirm all of this behavior:
* The first test is a new test that passes both before and after this
commit which performs a failed allocation of a memory slot. A
successful allocation is then made to ensure that the previous image
is not present and zero memory is present. This test fails before the
commit if the clear-on-drop behavior is removed, and it fails with this
commit if the clear-on-reusing-unknown behavior is removed.
Effectively this test ensures that the clear-on-unknown-state logic is
present.
* The second test is a new test that panicked before this commit and
passes afterwards. This second test exhausts all VMAs in the current
process, or at least most of them, and then tries to allocate some
instances with an image. Instance allocation will eventually fail and
cause the erroneous path to get executed. This previously unwrapped a
`ENOMEM` failure, and now it can be handled gracefully by the
embedder.
* Skip the new test on QEMU, it fails on CI
* Only run test on Linux
show more ...
|
| e1f50aad | 21-Aug-2025 |
Alex Crichton <[email protected]> |
Make table/memory creation async functions (#11470)
* Make core instance allocation an `async` function
This commit is a step in preparation for #11430, notably core instance
allocation, or `Store
Make table/memory creation async functions (#11470)
* Make core instance allocation an `async` function
This commit is a step in preparation for #11430, notably core instance
allocation, or `StoreOpaque::allocate_instance` is now an `async fn`.
This function does not actually use the `async`-ness just yet so it's a
noop from that point of view, but this propagates outwards to enough
locations that I wanted to split this off to make future changes more
digestable.
Notably some creation functions here such as making an `Instance`,
`Table`, or `Memory` are refactored internally to use this new `async`
function. Annotations of `assert_ready` or `one_poll` are used as
appropriate as well.
For reference this commit was benchmarked with our `instantiation.rs`
benchmark in the pooling allocator and shows no changes relative to the
original baseline from before-`async`-PRs.
* Make table/memory creation `async` functions
This commit is a large-ish refactor which is made possible by the many
previous refactorings to internals w.r.t. async-in-Wasmtime. The end
goal of this change is that table and memory allocation are both `async`
functions. Achieving this, however, required some refactoring to enable
it to work:
* To work with `Send` neither function can close over `dyn VMStore`.
This required changing their `Option<&mut dyn VMStore>` arugment to
`Option<&mut StoreResourceLimiter<'_>>`
* Somehow a `StoreResourceLimiter` needed to be acquired from an
`InstanceAllocationRequest`. Previously the store was stored here as
an unsafe raw pointer, but I've refactored this now so
`InstanceAllocationRequest` directly stores `&StoreOpaque` and
`Option<&mut StoreResourceLimiter>` meaning it's trivial to acquire
them. This additionally means no more `unsafe` access of the store
during instance allocation (yay!).
* Now-redundant fields of `InstanceAllocationRequest` were removed since
they can be safely inferred from `&StoreOpaque`. For example passing
around `&Tunables` is now all gone.
* Methods upwards from table/memory allocation to the
`InstanceAllocator` trait needed to be made `async`. This includes new
`#[async_trait]` methods for example.
* `StoreOpaque::ensure_gc_store` is now an `async` function. This
internally carries a new `unsafe` block carried over from before with
the raw point passed around in `InstanceAllocationRequest`. A future
PR will delete this `unsafe` block, it's just temporary.
I attempted a few times to split this PR up into separate commits but
everything is relatively intertwined here so this is the smallest
"atomic" unit I could manage to land these changes and refactorings.
* Shuffle `async-trait` dep
* Fix configured build
show more ...
|