search

rtomayko / posix-spawn

Ruby process spawning library
Other
519 stars 52 forks source link

Fix memory leak in env preparation #87

Closed uberjay closed 4 years ago

uberjay commented 4 years ago

Free the existing entry's memory when replacing an existing entry in the environment. In the calling function (rb_posixspawn_pspawn), the processes' current env is duplicated; each existing environment entry is copied with strdup.

This only occurs if posix_spawn is supplied with an environment hash containing keys which collide with existing keys in the environment. If that is the case, each conflicting entry's memory is leaked.

This was the source of a long-standing mysterious memory growth in a few of our services. 🎉

uberjay commented 4 years ago

I fixed the whitespace in posix-spawn.c; let me know if you'd like any other changes! This was tricky to track down, and I learned a lot in the process. Special thanks to @SamSaffron for pointing out mwrap and chap. Those two tools were invaluable in tracking this (and a couple other issues) down.

tmm1 commented 4 years ago

Very nice find. I have not tried those tools myself before, but they look very useful. Maybe you'll consider writing a blog post about your hunt here.

I'm going to merge this, and will see if I am able to release a new gem.

SamSaffron commented 4 years ago

Wow @uberjay so happy my blog post at https://samsaffron.com/archive/2019/10/08/debugging-unmanaged-and-hidden-memory-leaks-in-ruby helped out!