On Thu, Jun 3, 2010 at 8:40 PM, Eric Wong <[email protected]> wrote: > Augusto Becciu <[email protected]> wrote: >> On Wed, Jun 2, 2010 at 6:38 PM, Eric Wong <[email protected]> wrote: >> > Augusto Becciu <[email protected]> wrote: >> >> On Wed, Jun 2, 2010 at 5:25 PM, Eric Wong <[email protected]> wrote: >> >> > Augusto Becciu <[email protected]> wrote: >> >> >> Hey guys, >> >> >> >> >> >> Started running unicorn in a production server like two weeks ago. >> >> >> It's been running smoothly, but looking at the logs found 44 >> >> >> exceptions like this: >> >> >> >> >> >> E, [2010-06-02T16:17:15.117071 #22680] ERROR -- : Read error: >> >> >> #<TypeError: can't modify frozen string> >> >> >> E, [2010-06-02T16:17:15.117270 #22680] ERROR -- : >> >> >> /usr/lib/ruby/gems/1.8/gems/unicorn-0.99.0/lib/unicorn/http_request.rb:59:in >> >> >> `headers' >> >> > >> >> > <snip> >> >> > >> >> >> Ruby version: ruby 1.8.7 (2009-12-24 patchlevel 248) [i686-linux], >> >> >> MBARI 0x8770, Ruby Enterprise Edition 2010.01 > > <snip> > >> >> There's no way our application could be freezing that constant, at >> >> least not intentionally. > > <snip> > >> Thanks Eric! Unfortunately after completely disabling RPM, we keep >> getting that error. :( >> >> What else could it be? > > Any details about your application you're allowed to share can help us, > especially a list of library/gem dependencies and versions. > > I would grep through your gems and vendor directories to ensure there's > nothing freezing objects it shouldn't freeze. It could also be a buggy > C extension corrupting memory somewhere, too... > > This isn't happening for all your worker processes, is it? > > If your application logs show which PID handled each request, you can > join the PIDs from the application logs with PIDs in the error logs > generated by Unicorn. That lets you narrow down the possible requests > that could trigger the errors and help you reproduce the problem sooner. > > This is one of my favorite features of one-request-per-process model > is that you can easily narrow down which request is triggering a > particular bug without worrying about race conditions from other > requests. > > This is certainly the first time I've heard of this problem, so I think > it's specific to something in your application/environment. > > -- > Eric Wong >
Hi Eric, Thanks for the ideas. Ended up adding some debugging code to unicorn that lead me to the culprit of the problem. It turned out being a bug in the HttpParser. When Unicorn receives a request with a "Version" header, the HttpParser transforms it into "HTTP_VERSION". After that tries to add it to the request hash which already contains a "HTTP_VERSION" key with the actual http version of the request. So it tries to append the new value separated by a comma. But since the http version is a freezed constant, the TypeError exception is raised. According to the HTTP RFC (http://www.w3.org/Protocols/rfc2616/rfc2616-sec7.html#sec7.1) a "Version" header is valid. However, it's not supported in rack, since rack has a HTTP_VERSION env variable for the http version. So I think the easiest way to deal with this problem is to just ignore the header since it is extremely unusual. We were getting it from a crappy bot. Below is a proposed patch. Thanks a lot for your help! Augusto diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index 264db68..aa23024 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -197,6 +197,15 @@ static void write_value(VALUE req, struct http_parser *hp, assert_frozen(f); } + /* + * ignore "Version" headers since they conflict with the HTTP_VERSION + * rack env variable. + */ + if (rb_str_cmp(f, g_http_version) == 0) { + hp->cont = Qnil; + return; + } + e = rb_hash_aref(req, f); if (NIL_P(e)) { hp->cont = rb_hash_aset(req, f, v); diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb index a067ac8..cb30f32 100644 --- a/test/unit/test_http_parser_ng.rb +++ b/test/unit/test_http_parser_ng.rb @@ -440,4 +440,24 @@ class HttpParserNgTest < Test::Unit::TestCase end end + def test_ignore_version_header + http = "GET / HTTP/1.1\r\nVersion: hello\r\n\r\n" + req = {} + assert_equal req, @parser.headers(req, http) + assert_equal '', http + expect = { + "SERVER_NAME" => "localhost", + "rack.url_scheme" => "http", + "REQUEST_PATH" => "/", + "SERVER_PROTOCOL" => "HTTP/1.1", + "PATH_INFO" => "/", + "HTTP_VERSION" => "HTTP/1.1", + "REQUEST_URI" => "/", + "SERVER_PORT" => "80", + "REQUEST_METHOD" => "GET", + "QUERY_STRING" => "" + } + assert_equal expect, req + end + end _______________________________________________ Unicorn mailing list - [email protected] http://rubyforge.org/mailman/listinfo/mongrel-unicorn Do not quote signatures (like this one) or top post when replying
