Graceful Handling of OOM in table.grow
#5394
Open
Conversation
This file contains 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
This commit modifies the grow method in VMTable to handle
out-of-memory (OOM) errors gracefully by using fallible memory
reservation. Previously, Vec::resize would panic on OOM, causing a
crash instead of returning -1 as required by the WebAssembly
specification. The changes ensure that:
1. Fallible Memory Reservation
* "try_reserve_exact" is used to check if the system can allocate
the required memory before resizing.
* If allocation fails, the method returns None, adhering to the
spec.
2. Safe Resize
* If memory reservation succeeds, resize is performed safely
without panicking
This fix addresses issues where large allocations (e.g.,
0xff_ff_ff_ff) would crash on Linux due to stricter memory allocation
checks, while macOS's virtual memory overcommit might mask the
problem.
Signed-off-by: Charalampos Mitrodimas <[email protected]>
Adds a new test to verify table.grow behavior with very large delta (0xff_ff_ff_ff) across different platforms. On Linux this should return -1, while on macOS it returns 0. The test includes platform-specific assertions to verify this behavior.
syrusakbary
reviewed
Feb 17, 2025
Comment on lines
+35
to
+47
| #[cfg(target_os = "macos")] | ||
| assert_eq!( | ||
| result[0], | ||
| wasmer::Value::I32(0), | ||
| "On macOS, table.grow should return 0" | ||
| ); | ||
|
|
||
| #[cfg(target_os = "linux")] | ||
| assert_eq!( | ||
| result[0], | ||
| wasmer::Value::I32(-1), | ||
| "On Linux, table.grow should return -1" | ||
| ); |
There was a problem hiding this comment.
This shouldn't happen (different output for same program, under different OSs). Good catch.
We need to make the API deterministic no matter what OS is running on
There was a problem hiding this comment.
Not sure/haven't put more thought into whether this is possible.
Pasting the scenarios here (before this PRs changes), since I have them handy, for some food for thought.
Linux:
- Linux with
vm.overcommit_memory = 0results inmemory allocation of ... bytes failed - Linux with
vm.overcommit_memory = 1results inkilled - Linux with
vm.overcommit_memory = 2results inmemory allocation of ... bytes failed
MacOS always works without exiting.
Maybe there's not much we can do here. But doing nothing will not adere the specification (i.e. The table.grow instruction grows table by a given delta and returns the previous size, or −1 if enough space cannot be allocated).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This commit modifies the grow method in VMTable to handle out-of-memory (OOM) errors gracefully by using fallible memory reservation. Previously,
Vec::resizewould panic on OOM, causing a crash instead of returning -1 as required by the WebAssembly specification. The changes ensure that:This fix addresses issues where large allocations (e.g., 0xff_ff_ff_ff) would crash on Linux due to stricter memory allocation checks, while macOS's virtual memory overcommit might mask the problem.
Closes #5297