-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix WASM memory allocation failure (#4989) #7887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3ae73f0
09aba03
2808d1f
f084aba
2be3382
4c4f4ee
620f31d
eeb0dad
74b95a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,3 +6,19 @@ rustflags = "-C link-args=-Wl,--stack,8000000" | |
|
|
||
| [target.wasm32-unknown-unknown] | ||
| rustflags = ["--cfg=getrandom_backend=\"wasm_js\""] | ||
|
|
||
| # Enforce a 64 MB memory cap and 1 MB stack limit for WASI targets. | ||
| # Without these, the WASM heap can grow unboundedly (e.g., a simple | ||
| # `a = 1` allocated ~79 MB) and crash on constrained runtimes like wasmi. | ||
| # See issue #4989. | ||
| [target.wasm32-wasip1] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add a comment for the reasoning behind those rustflags?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, done! I’ve added a brief comment explaining why these limits are necessary, referencing the original issue. Let me know if you’d like more details. |
||
| rustflags = [ | ||
| "-C", "link-arg=--max-memory=67108864", | ||
| "-C", "link-arg=-zstack-size=1048576", | ||
| ] | ||
|
|
||
| [target.wasm32-wasip2] | ||
| rustflags = [ | ||
| "-C", "link-arg=--max-memory=67108864", | ||
| "-C", "link-arg=-zstack-size=1048576", | ||
| ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ | |
| flamescope = { version = "0.1.2", optional = true } | ||
|
|
||
| rustls = { workspace = true, optional = true } | ||
| rustls-graviola = { workspace = true, optional = true } | ||
|
|
||
| [target.'cfg(windows)'.dependencies] | ||
| libc = { workspace = true } | ||
|
|
@@ -117,6 +117,14 @@ | |
| [profile.release] | ||
| lto = "thin" | ||
|
|
||
| [profile.wasm-release] | ||
| inherits = "release" | ||
| opt-level = "s" | ||
| lto = true | ||
| codegen-units = 1 | ||
| strip = true | ||
| panic = "abort" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not so thrilled about adding another build profile, but I can see where it would be useful.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main reason I added wasm-release is just to make the WASM file as small as possible. This kind of setup is pretty common (you see it in wasm-pack, Yew, and other WASM projects).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wasm-pack and Yew are projects that are aimed to be ran only on wasm, which is not the case for RustPython.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A better example for a project that does WASM on the side is swc (the Rust JS/TS compiler). It's mostly a CLI tool, but they also ship a WASM build for browser playgrounds. In their Cargo.toml they have a separate [profile.wasm] with similar size tweaks.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't see it there. What I do see is this: https://github.com/swc-project/swc/blob/a3f23b10986654bcc7296283786d90934a39a53b/Cargo.toml#L165-L182
I don't see them defining a profile for wasm at: https://github.com/astral-sh/ruff/blob/6aaa91ac2b269df1414954ccd5134f0e6f5c6d30/Cargo.toml can you please point me to the exact place you saw them defining a custom profile for wasm?
I don't have an issue with adding another profile. I'm just not experienced with WASM, I want to see what is the best choice before choosing an approach.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I spent some time digging through projects that are in a similar boat as RustPython (because example of swc isn't seems well which i though for but)— projects that are primarily native but also support WASM as an extra target. What stood out is that several of them do exactly what I'm suggesting here: a separate [profile.wasm-release] so the standard release profile stays untouched for native builds. Let me share the ones I found most relevant which i find while digging up : Ruffle — the Flash emulator. It's a desktop application at its core, but they also build for the browser. To keep things clean, they define separate profiles like [profile.web-wasm-mvp] and [profile.web-wasm-extensions] for the WASM targets, leaving the native release profile alone. dogoap — a GOAP AI library that isn't a WASM project. Yet they have a [profile.gold-release] for native builds and a [profile.wasm-release] for WASM demos sitting right next to each other. Exactly the same split I'm proposing. another-boids-in-rust — a Bevy game that runs on both desktop and WASM. Their [profile.wasm-release] uses opt-level = "z", lto = true, codegen-units = 1, strip = "symbols", and panic = "abort" — settings that are basically identical to what I've added here. All these projects treat WASM as a feature, not the main thing. And in every case, the dedicated profile is there simply to make sure the native build remains unaffected, while WASM gets the size‑conscious settings it needs. That's the exact situation we have with RustPython — it's a native interpreter first, WASM is an extra target. Adding a [profile.wasm-release] feels like the cleanest way to ship a small WASM binary without asking every contributor to remember manual compiler flags.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, thanks for the info! Can you adjust the CI to use this profile?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! The CI now uses -profile wasm-release for the WASM build and tests. All 26 checks are green, including the wasm-wasi job. |
||
|
|
||
| [patch.crates-io] | ||
| parking_lot_core = { git = "https://github.com/youknowone/parking_lot", branch = "rustpython" } | ||
| # REDOX START, Uncomment when you want to compile/check with redoxer | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understood how 1MB stack limit helps the problem, but not about 64MB memory. does limiting memory actually help memory grow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without a max memory limit, the WASM runtime doesn't know when to stop. It can keep taking more and more memory until it crashes — even for a tiny script.
The 64 MB cap acts like a boundary. It tells the runtime: "you can't cross this line." So instead of spiraling to 79 MB and crashing, it stays safely within 64 MB.
So the cap doesn't make memory grow — it prevents runaway growth. That's why it helps.
Hope that makes sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would it need 79MB in the first place if it would do it with 64MB 🤔
I'm missing something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
79 MB was what I actually observed before the fix, it's the point where the runtime crashed because no limit was set. The script didn't need that much, but without a cap, the heap just kept growing until it fell over. Now with the 64 MB cap, it stays within that boundary and runs fine.
it was just the runtime grabbing more and more memory without a stop sign. The 64 MB cap says "this far, no further", and since the real work fits easily under that, the script runs fine. The cap just stops the runaway growth, it doesn't squeeze anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am really lost how it goes 79MB if it actaully can run under 64MB.
So you mean, even after reducing the stack size, it still takes 79MB without 64MB limit. right?
I am not wasm runtime pro. I'd better to expriment about this.
But 64MB hard limit sounds like it will not allow more memory even when it actually requires 100MB memory.
Do you have recommendation any kind of reference i can read about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
• WASM spec — memory
maxfield:https://webassembly.github.io/spec/core/syntax/types.html#memory-types
• Rust Wasm book — linker flags & memory:
https://rustwasm.github.io/book/reference/code-size.html
Honestly, the links Above 👆 explain the mechanics much better
than I can in a comment(They're quick reads and should make it click in mind)
79 MB crash happened before we fixed the chunk size and profiling issues. After those fixes, the program only needs a few megabytes of memory in normal operation.
However, without a memory limit, the runtime can still occasionally request much more memory at once, such as when it decides to expand the heap. In those cases, memory usage could exceed 64 MB and cause another crash.
The 64 MB limit is simply a safeguard to prevent those sudden memory spikes from pushing the program over the available memory and crashing again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to ask what do you do if you actually need more than 64MB. if the core interpreter spend about 64MB memory, loading data and more code can spend more than 64MB easy. then what's the proper answer for users require more than 64MB+?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me explain 2 of big thing i guess you may not understand first is:
About The 79 MB crash happened before we applied any of the fixes.
At that time, the code still had a big DataStack chunk and no size optimization. That made the runtime grab way more memory than needed.
Then applied the other two fixes:
After those changes, SECOND THING is the script actually needs much less memory — easily under 64 MB.
So why add the 64 MB cap?
Even when the script only needs a little memory, the WASM runtime sometimes asks for memory in large chunks (like doubling the heap). Without a limit, it could still overshoot to 80 MB or more and crash. The 64 MB cap tells the runtime: “you are never allowed to go past this.” Since the real need is far below that, the runtime never tries to overshoot, and the crash is gone.
please cheak the links in previous comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
64 MB is just a starting number — it's meant for the kind of small-to-medium scripts you'd run in a limited WASM environment. If someone needs more memory later, they don't have to change any code. They just pass a different flag when building.
For example, a user can build with:
cargo build --profile wasm-release --target wasm32-wasi --config "target.wasm32-wasi.rustflags=['-C', 'link-arg=--max-memory=134217728']"
That bumps the limit to 128 MB right there, no code changes needed.
The main thing is that without any limit, even a tiny script can crash because the runtime grabs memory too aggressively. The cap just stops that runaway behavior. And since the number is easy to adjust, anyone who really needs more memory can simply raise it. It's a safety net, not a permanent restriction.