summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2011-01-05 23:05:05 -0800
committerEric Wong <normalperson@yhbt.net>2011-01-05 23:05:05 -0800
commitb7a0074284d33352bb9e732c660b29162f34bf0e (patch)
tree664d6b3f1f732b597c41f1b48a5f478ce197de71
parent7f3ebe9213e09932cd0e8a2a82bfe2dd5430a824 (diff)
Response bodies may capture the block passed to each
and save it for body.close, so don't close the socket
before we have a chance to call body.close

(cherry picked from commit b72a86f66c722d56a6d77ed1d2779ace6ad103ed)

Conflicts:

	lib/unicorn/http_server.rb
	test/unit/test_response.rb
-rw-r--r--lib/unicorn.rb1
-rw-r--r--lib/unicorn/http_response.rb1
-rwxr-xr-xt/t0018-write-on-close.sh23
-rw-r--r--t/write-on-close.ru11
-rw-r--r--test/unit/test_response.rb21
5 files changed, 45 insertions, 12 deletions
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 735354f..31332c9 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -646,6 +646,7 @@ module Unicorn
         response = app.call(env)
       end
       HttpResponse.write(client, response, HttpRequest::PARSER.headers?)
+      client.close # flushes and uncorks the socket immediately, no keepalive
     rescue => e
       handle_error(client, e)
     end
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index 6f1cd48..f3b5a82 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -63,7 +63,6 @@ module Unicorn::HttpResponse
     end
 
     body.each { |chunk| socket.write(chunk) }
-    socket.close # flushes and uncorks the socket immediately
     ensure
       body.respond_to?(:close) and body.close
   end
diff --git a/t/t0018-write-on-close.sh b/t/t0018-write-on-close.sh
new file mode 100755
index 0000000..3afefea
--- /dev/null
+++ b/t/t0018-write-on-close.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+. ./test-lib.sh
+t_plan 4 "write-on-close tests for funky response-bodies"
+
+t_begin "setup and start" && {
+        unicorn_setup
+        unicorn -D -c $unicorn_config write-on-close.ru
+        unicorn_wait_start
+}
+
+t_begin "write-on-close response body succeeds" && {
+        test xGoodbye = x"$(curl -sSf http://$listen/)"
+}
+
+t_begin "killing succeeds" && {
+        kill $unicorn_pid
+}
+
+t_begin "check stderr" && {
+        check_stderr
+}
+
+t_done
diff --git a/t/write-on-close.ru b/t/write-on-close.ru
new file mode 100644
index 0000000..54a2f2e
--- /dev/null
+++ b/t/write-on-close.ru
@@ -0,0 +1,11 @@
+class WriteOnClose
+  def each(&block)
+    @callback = block
+  end
+
+  def close
+    @callback.call "7\r\nGoodbye\r\n0\r\n\r\n"
+  end
+end
+use Rack::ContentType, "text/plain"
+run(lambda { |_| [ 200, [%w(Transfer-Encoding chunked)], WriteOnClose.new ] })
diff --git a/test/unit/test_response.rb b/test/unit/test_response.rb
index f9eda8e..b3bc3a2 100644
--- a/test/unit/test_response.rb
+++ b/test/unit/test_response.rb
@@ -15,15 +15,14 @@ class ResponseTest < Test::Unit::TestCase
   def test_response_headers
     out = StringIO.new
     HttpResponse.write(out,[200, {"X-Whatever" => "stuff"}, ["cool"]])
-    assert out.closed?
-
+    assert ! out.closed?
     assert out.length > 0, "output didn't have data"
   end
 
   def test_response_string_status
     out = StringIO.new
     HttpResponse.write(out,['200', {}, []])
-    assert out.closed?
+    assert ! out.closed?
     assert out.length > 0, "output didn't have data"
     assert_equal 1, out.string.split(/\r\n/).grep(/^Status: 200 OK/).size
   end
@@ -33,7 +32,7 @@ class ResponseTest < Test::Unit::TestCase
     $, = "\f\v"
     out = StringIO.new
     HttpResponse.write(out,[200, {"X-k" => "cd","X-y" => "z"}, ["cool"]])
-    assert out.closed?
+    assert ! out.closed?
     resp = out.string
     assert ! resp.include?("\f\v"), "output didn't use $, ($OFS)"
     ensure
@@ -43,7 +42,7 @@ class ResponseTest < Test::Unit::TestCase
   def test_response_200
     io = StringIO.new
     HttpResponse.write(io, [200, {}, []])
-    assert io.closed?
+    assert ! io.closed?
     assert io.length > 0, "output didn't have data"
   end
 
@@ -51,7 +50,7 @@ class ResponseTest < Test::Unit::TestCase
     code = 400
     io = StringIO.new
     HttpResponse.write(io, [code, {}, []])
-    assert io.closed?
+    assert ! io.closed?
     lines = io.string.split(/\r\n/)
     assert_match(/.* Bad Request$/, lines.first,
                  "wrong default reason phrase")
@@ -60,7 +59,7 @@ class ResponseTest < Test::Unit::TestCase
   def test_rack_multivalue_headers
     out = StringIO.new
     HttpResponse.write(out,[200, {"X-Whatever" => "stuff\nbleh"}, []])
-    assert out.closed?
+    assert ! out.closed?
     assert_match(/^X-Whatever: stuff\r\nX-Whatever: bleh\r\n/, out.string)
   end
 
@@ -69,7 +68,7 @@ class ResponseTest < Test::Unit::TestCase
   def test_status_header_added
     out = StringIO.new
     HttpResponse.write(out,[200, {"X-Whatever" => "stuff"}, []])
-    assert out.closed?
+    assert ! out.closed?
     assert_equal 1, out.string.split(/\r\n/).grep(/^Status: 200 OK/i).size
   end
 
@@ -80,7 +79,7 @@ class ResponseTest < Test::Unit::TestCase
     out = StringIO.new
     header_hash = {"X-Whatever" => "stuff", 'StaTus' => "666" }
     HttpResponse.write(out,[200, header_hash, []])
-    assert out.closed?
+    assert ! out.closed?
     assert_equal 1, out.string.split(/\r\n/).grep(/^Status: 200 OK/i).size
     assert_equal 1, out.string.split(/\r\n/).grep(/^Status:/i).size
   end
@@ -91,7 +90,7 @@ class ResponseTest < Test::Unit::TestCase
     body.rewind
     out = StringIO.new
     HttpResponse.write(out,[200, {}, body])
-    assert out.closed?
+    assert ! out.closed?
     assert body.closed?
     assert_match(expect_body, out.string.split(/\r\n/).last)
   end
@@ -99,7 +98,7 @@ class ResponseTest < Test::Unit::TestCase
   def test_unknown_status_pass_through
     out = StringIO.new
     HttpResponse.write(out,["666 I AM THE BEAST", {}, [] ])
-    assert out.closed?
+    assert ! out.closed?
     headers = out.string.split(/\r\n\r\n/).first.split(/\r\n/)
     assert %r{\AHTTP/\d\.\d 666 I AM THE BEAST\z}.match(headers[0])
     status = headers.grep(/\AStatus:/i).first