This repository was archived by the owner on Mar 24, 2022. It is now read-only.
replace terminate_on_heap_oom with wasmtime-like MemoryLimiter#682
Merged
Conversation
added 6 commits
October 25, 2021 16:16
these will be replaced with a wasmtime-style MemoryLimiter
just an async one, because i dont care about synchronous users anymore, they can use wasmtime
2bd1ff8 to
bec1320
Compare
Contributor
|
Okay, just to make sure I understand, we will no longer get a runtime error back from |
Contributor
Author
|
Yes, exactly that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lucet's
Instance::terminate_on_heap_oombehavior was, from the beginning (#583), something that deviated from the Wasm spec. This PR eliminates it in favor of a different mechanism,MemoryLimiter.MemoryLimiter::memory_growingprovides users a mechanism to allow or disallow amemory.growoperation, as well asawaiton memory becoming available.terminate_on_heap_ppmis replaced by theMemoryLimiter::memory_grow_failedmethod, where an unsuccessful grow (whether due tomemory_growing,Limits, or an OS memory allocation failure) is reported to the embedder.When we went to add heap-out-of-memory detection in Wasmtime, we had to figure out a way to do so without breaking the spec, and that turned into
ResourceLimiter.Here in Lucet, I introduced
MemoryLimiter, which only does the memory-related operations of wasmtime's ResourceLimiter.Unlike in Wasmtime, where we went to some pains to make it work with both sync and async embeddings, this embedding only works for async, because that is what Fastly's embedding needs and we aren't putting energy into generalizing the features.
Also unlike in Wasmtime, the
MemoryLimiterdoesn't attempt to restrict memory operations by the embedding API (e.g.Instance::grow_memory), because we don't end up needing it in the Fastly embedding. TheMemoryLimiteronly works on wasmmemory.growoperations.