typhoeus/ethon — pull request #45 by richievos (2013-04-08 21:47:18+00:00)
Title: Fix memory leaks

This pull request fixes a bunch of issues. The primary one is it fixes the large memory leaks in Ethon (which propagate up to Typhoeus). Specifically, we at Groupon use Typhoeus for some internal HTTP calls, and in upgrading typhoeus (0.2.3 to 0.6.2) we noticed some very significant memory leaks.

After doing an analysis of the code (and reading up on FFI/finalizers), I found that none of the finalizers in Ethon were working properly. Because of the way they worked, they all were causing memory leaks. The version in this pull request redoes those, and leverages FFI::AutoPointer to do the cleanup (that is what AutoPointer is for). Switching to AutoPointer removes the need for this gem to use Objectspace, which in theory will make it behave better in JRuby.

This also fixes some issues where memory that was getting created by curl wasn't ever getting cleaned up. The largest issue was in the curl_escape call (Curl.escape). That was initializing C memory, but never actually recollecting it.

Finally, I've also created a test which looks for memory leaks. I don't know if it's something that should be added to the automated build (it still gets some false positives), but it's very useful for checking for memory leaks. From the commit message:

commit 8849db5b223eb9fd14727cc79be8d3e1eacb7c3a
Author: Richie Vos
Date: Sun Apr 7 21:26:48 2013 -0500

Add a memory leak testing script

I've been manually running this as:

RUBY_HEAP_SLOTS_GROWTH_FACTOR=1 RUBY_HEAP_MIN_SLOTS=1 RUBY_HEAP_SLOTS_INCREMENT=10 RUBY_HEAP_SLOTS_GROWTH_FACTOR=.1 ITERATIONS=75000 rspec -I lib -I profile profile/memory_leaks.rb

The ruby GC settings are that way to try and make ruby increase memory really
slowly, so that we can get more accurate info on what's going on.

ITERATIONS can be whatever, but I've been running it with a really large number
so that I can be more comfortable with the results.

The commits themselves reference the issues that have been fixed/touched as part of this, but I'll copy them here for longevity:

fixes typhoeus/ethon#43
fixes typhoeus/ethon#44
affects typhoeus/ethon#30
fixes typhoeus/ethon#34
fixes typhoeus/typhoeus#229

This branch is based off of v0.5.10 since master has failing tests.

I personally would really appreciate a quick merge on this, and also a patch release. Preferably v0.5.11 would include just these changes, and there'd be a corresponding Typhoeus release.