Skip to content
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
pchickey merged 6 commits into
mainfrom
pch/memory_limiter
Oct 27, 2021
Merged

replace terminate_on_heap_oom with wasmtime-like MemoryLimiter#682
pchickey merged 6 commits into
mainfrom
pch/memory_limiter

Conversation

@pchickey
Copy link
Copy Markdown
Contributor

@pchickey pchickey commented Oct 25, 2021

Lucet's Instance::terminate_on_heap_oom behavior 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_growing provides users a mechanism to allow or disallow a memory.grow operation, as well as await on memory becoming available. terminate_on_heap_ppm is replaced by the MemoryLimiter::memory_grow_failed method, where an unsuccessful grow (whether due to memory_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 MemoryLimiter doesn'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. The MemoryLimiter only works on wasm memory.grow operations.

Pat Hickey added 6 commits October 25, 2021 16:16
@acfoltzer
Copy link
Copy Markdown
Contributor

Okay, just to make sure I understand, we will no longer get a runtime error back from Instance::run_async when the heap is exhausted. Instead, we'll set up some mechanism in a MemoryLimiter impl that keeps track of whether the heap has been exhausted, and then check+translate that into an embedder-specific error after the run completes?

@pchickey
Copy link
Copy Markdown
Contributor Author

Yes, exactly that.

Copy link
Copy Markdown
Contributor

@acfoltzer acfoltzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, sounds good!

@pchickey pchickey merged commit 68ce2aa into main Oct 27, 2021
@pchickey pchickey deleted the pch/memory_limiter branch October 27, 2021 17:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants