search

matsadler / magnus

Ruby bindings for Rust. Write Ruby extension gems in Rust, or call Ruby from Rust.
https://docs.rs/magnus/latest/magnus/
MIT License
682 stars 35 forks source link

Fix memory leak in `error::raise` #30

Closed ianks closed 2 years ago

ianks commented 2 years ago

When running ruby_memcheck against wasmtime-rb's test suite, and Valgrind detected a bunch of small leaks coming from `error::raise.

Turns out, rb_raise never frees the error message buffer so the memory for every raised Error::Error leaks.

To fix this, error:raise now uses rb_exc_new_str which will allow the Ruby GC to free the RString error message. With this change, Valgrind no longer detects the leak.

matsadler commented 2 years ago

Oh wow, I went to such great lengths to avoid leaks when raising errors (i.e. the whole design of returning errors to raise), and then right there at the very last hurdle there's a leak just staring me in the face.

I took a slightly different tack and (assuming no further oversights) fixed this in 8c870e3.

ianks commented 2 years ago

Awesome, thanks @matsadler. Seems to resolve the issue.