Skip to content

Fix jump buffer leak in WASI builds #13142

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

Merged
merged 2 commits into from
Apr 27, 2025

Conversation

white-axe
Copy link
Contributor

This pull request fixes a memory leak in WASI (WebAssembly) builds of Ruby caused by the VM jump buffers not being freed in certain parts of the code.

I originally attempted to fix this memory leak in #12995 (it's the second memory leak mentioned in that pull request), but the fix was reverted by #13026 due to test failures. The fix has been rewritten to be much simpler and less invasive so that it has less of a chance of breaking things.

I've already tested building with this patch here: https://github.com/white-axe/ruby.wasm/actions/runs/14563460222/job/40849624514

For ease of reference, I'm mentioning again here that there's a test to determine if a WASI build of Ruby is affected by the two memory leaks mentioned in #12995:

Here's a test to see if a WASI build is affected by these two memory leaks. This script causes all of the current WASI builds from https://github.com/ruby/ruby.wasm to rapidly consume around 800 mebibytes of memory due to the jump buffer leak and then crash from what I believe is stack buffer overflow caused by the stack pointer leak:

loop do
  array = [0]
  for item in array
    break
  end
end

To test it out, save it as test.rb in the current working directory, and then:

curl -Lo ruby.tar.gz https://github.com/ruby/ruby.wasm/releases/download/2.7.1/ruby-3.4-wasm32-unknown-wasip1-full.tar.gz
tar xzf ruby.tar.gz --strip-components=1
wasmtime --dir .::/ usr/local/bin/ruby test.rb

Note: The first memory leak has been fixed in the master branch so it won't immediately crash from stack buffer overflow anymore, but the test still crashes from running out of memory due to the second memory leak.

Requesting a review from @kateinoigakukun.

@kateinoigakukun
Copy link
Member

If I understand correctly, we still need to deinit jump buffers for intermediate frames as you did in the latest fix, shouldn't we?

@white-axe
Copy link
Contributor Author

Yeah, sorry, I forgot about one case where the longjmp caused by calling a continuation also leaks jump buffers, which I just fixed. Here's a test to see if continuations leak jump buffers:

require 'continuation'
callcc {|cc| $cc = cc}
array = [0]
for item in array
  $cc.call
end

Other than that, though, apparently all of the other longjmps in the Ruby code are made from inside of calls to EC_JUMP_TAG(), which just jumps to GET_EC()->tag->buf, i.e. it never skips over any tags, so no intermediate jump buffers are ever actually leaked by any of the longjmps other than the one in cont.c.

Jumping to a different fiber also doesn't leak jump buffers because the fiber's deinitialization will free the jump buffers that belong to the fiber, as far as I can tell.

Copy link

launchable-app bot commented Apr 21, 2025

All Tests passed!

✖️no tests failed ✔️61715 tests passed(1 flake)

Copy link
Member

@kateinoigakukun kateinoigakukun left a comment

Choose a reason for hiding this comment

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

Sorry for taking a long time to review. It looks good to me, huge thanks!

@kateinoigakukun kateinoigakukun merged commit 6874b28 into ruby:master Apr 27, 2025
84 checks passed
@white-axe white-axe deleted the wasi-jumpbuf-leak branch April 27, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants