summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2010-02-12 23:59:55 -0800
committerEric Wong <normalperson@yhbt.net>2010-02-13 00:34:50 -0800
commitafdc6fce5f7e34a5c07f24204625cd466e93e5ac (patch)
tree1dfb1c5d5e0dccae3480609147b6e7dfb1226b48
parent615cede23579906b486c7a18bdeb22d04864a6ea (diff)
First off, this memory leak DOES NOT affect Unicorn itself.
Unicorn allocates the HttpParser once and always reuses it
in every sequential request.

This leak affects applications which repeatedly allocate a new
HTTP parser.  Thus this bug affects _all_ deployments of
Rainbows! and Zbatery.  These servers allocate a new parser for
every client connection.

I misread the Data_Make_Struct/Data_Wrap_Struct documentation
and ended up passing NULL as the "free" argument instead of -1,
causing the memory to never be freed.

From README.EXT in the MRI source which I misread:
> The free argument is the function to free the pointer
> allocation.  If this is -1, the pointer will be just freed.
> The functions mark and free will be called from garbage
> collector.
-rw-r--r--ext/unicorn_http/unicorn_http.rl2
-rw-r--r--test/unit/test_http_parser.rb18
2 files changed, 18 insertions, 2 deletions
diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
index 6232e2c..a95224c 100644
--- a/ext/unicorn_http/unicorn_http.rl
+++ b/ext/unicorn_http/unicorn_http.rl
@@ -439,7 +439,7 @@ static void hp_mark(void *ptr)
 static VALUE HttpParser_alloc(VALUE klass)
 {
   struct http_parser *hp;
-  return Data_Make_Struct(klass, struct http_parser, hp_mark, NULL, hp);
+  return Data_Make_Struct(klass, struct http_parser, hp_mark, -1, hp);
 }
 
 
diff --git a/test/unit/test_http_parser.rb b/test/unit/test_http_parser.rb
index 0443b46..5b0ca9f 100644
--- a/test/unit/test_http_parser.rb
+++ b/test/unit/test_http_parser.rb
@@ -527,5 +527,21 @@ class HttpParserTest < Test::Unit::TestCase
     end
 
   end
-end
 
+  # so we don't  care about the portability of this test
+  # if it doesn't leak on Linux, it won't leak anywhere else
+  # unless your C compiler or platform is otherwise broken
+  LINUX_PROC_PID_STATUS = "/proc/self/status"
+  def test_memory_leak
+    match_rss = /^VmRSS:\s+(\d+)/
+    if File.read(LINUX_PROC_PID_STATUS) =~ match_rss
+      before = $1.to_i
+      1000000.times { Unicorn::HttpParser.new }
+      File.read(LINUX_PROC_PID_STATUS) =~ match_rss
+      after = $1.to_i
+      diff = after - before
+      assert(diff < 10000, "memory grew more than 10M: #{diff}")
+    end
+  end if RUBY_PLATFORM =~ /linux/ && test(?r, LINUX_PROC_PID_STATUS)
+
+end