unicorn is an HTTP server for Rack applications designed to only serve fast clients on low-latency, high-bandwidth connections and take advantage of features in Unix/Unix-like kernels. Slow clients should only be served by placing a reverse proxy capable of fully buffering both the the request and response in between unicorn and slow clients. * https://bogomips.org/unicorn/ * public list: unicorn-public@bogomips.org * mail archives: https://bogomips.org/unicorn-public/ * git clone git://bogomips.org/unicorn.git * https://bogomips.org/unicorn/NEWS.atom.xml * nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn Changes: unicorn 5.3.0 A couple of portability fixes from Dylan Thacker-Smith and Jeremy Evans since 5.3.0.pre1 over a week ago, but this looks ready for a stable release, today. When I started this over 8 years ago, I wondered if this would just end up being an April Fools' joke. Guess not. I guess I somehow tricked people into using a terribly marketed web server that cannot talk directly to untrusted clients :x Anyways, unicorn won't be able to handle slow clients 8 years from now, either, or 80 years from now. And I vow never to learn to use new-fangled things like epoll, kqueue, or threads :P Anyways, this is a largish release with several new features, and no backwards incompatibilities. Simon Eskildsen contributed heavily using TCP_INFO under Linux to implement the (now 5 year old) check_client_connection feature: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-check_client_connection https://bogomips.org/unicorn-public/?q=s:check_client_connection&d:..20170401&x=t This also led to FreeBSD and OpenBSD portability improvements in one of our dependencies, raindrops: https://bogomips.org/raindrops-public/20170323024829.GA5190@dcvr/T/#u Jeremy Evans contributed several new features. First he implemented after_worker_exit to aid debugging: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-after_worker_exit https://bogomips.org/unicorn-public/?q=s:after_worker_exit&d:..20170401&x=t#t And then security-related features to isolate workers. Workers may now chroot to drop access to the master filesystem, and the new after_worker_ready configuration hook now exists to aid with chroot support in workers: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-after_worker_ready https://bogomips.org/unicorn/Unicorn/Worker.html#method-i-user https://bogomips.org/unicorn-public/?q=s:after_worker_ready&d:..20170401&x=t#t https://bogomips.org/unicorn-public/?q=s:chroot&d:..20170401&x=t#t Additionally, workers may run in a completely different VM space (nullifying preload_app and any CoW savings) with the new worker_exec option: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-worker_exec https://bogomips.org/unicorn-public/?q=s:worker_exec&d:..20170401&x=t#t There are also several improvements to FreeBSD and OpenBSD support with the addition of these features. shortlog of changes since v5.2.0 (2016-10-31): Dylan Thacker-Smith (1): Check for Socket::TCP_INFO constant before trying to get TCP_INFO Eric Wong (30): drop rb_str_set_len compatibility replacement TUNING: document THP caveat for Linux users tee_input: simplify condition for IO#write remove response_start_sent http_request: freeze constant strings passed IO#write Revert "remove response_start_sent" t/t0012-reload-empty-config.sh: access ivars directly if needed t0011-active-unix-socket.sh: fix race condition in test new test for check_client_connection revert signature change to HttpServer#process_client support "struct tcp_info" on non-Linux and Ruby 2.2+ unicorn_http: reduce rb_global_variable calls oob_gc: rely on opt_aref_with optimization on Ruby 2.2+ http_request: reduce insn size for check_client_connection freebsd: avoid EINVAL when setting accept filter test-lib: expr(1) portability fix tests: keep disabled tests defined test_exec: SO_KEEPALIVE value only needs to be true doc: fix links to raindrops project http_request: support proposed Raindrops::TCP states on non-Linux ISSUES: expand on mail archive info + subscription disclaimer test_ccc: use a pipe to synchronize test doc: remove private email support address input: update documentation and hide internals. http_server: initialize @pid ivar gemspec: remove olddoc from build dependency doc: add version annotations for new features unicorn 5.3.0.pre1 doc: note after_worker_exit is also 5.3.0+ test_exec: SO_KEEPALIVE value only needs to be true (take #2) Jeremy Evans (7): Add after_worker_exit configuration option Fix code example in after_worker_exit documentation Add support for chroot to Worker#user Add after_worker_ready configuration option Add worker_exec configuration option Don't pass a block for fork when forking workers Check for SocketError on first ccc attempt Simon Eskildsen (1): check_client_connection: use tcp state on linux -- Yes, this release is real despite the date.
unicorn is an HTTP server for Rack applications designed to only serve fast clients on low-latency, high-bandwidth connections and take advantage of features in Unix/Unix-like kernels. Slow clients should only be served by placing a reverse proxy capable of fully buffering both the the request and response in between unicorn and slow clients. * https://bogomips.org/unicorn/ * public list: unicorn-public@bogomips.org * mail archives: https://bogomips.org/unicorn-public/ * git clone git://bogomips.org/unicorn.git * https://bogomips.org/unicorn/NEWS.atom.xml * nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn This is a pre-release RubyGem intended for testing. Changes: unicorn 5.3.0.pre1 A largish release with several new features. Simon Eskildsen contributed heavily using TCP_INFO under Linux to implement the (now 5 year old) check_client_connection feature: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-check_client_connection https://bogomips.org/unicorn-public/?q=s:check_client_connection&d:..20170324&x=t This also led to FreeBSD and OpenBSD portability improvements in one of our dependencies, raindrops: https://bogomips.org/raindrops-public/20170323024829.GA5190@dcvr/T/#u Jeremy Evans contributed several new features. First he implemented after_worker_exit to aid debugging: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-after_worker_exit https://bogomips.org/unicorn-public/?q=s:after_worker_exit&d:..20170324&x=t#t And then security-related features to isolate workers. Workers may now chroot to drop access to the master filesystem, and the new after_worker_ready configuration hook now exists to aid with chroot support in workers: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-after_worker_ready https://bogomips.org/unicorn/Unicorn/Worker.html#method-i-user https://bogomips.org/unicorn-public/?q=s:after_worker_ready&d:..20170324&x=t#t https://bogomips.org/unicorn-public/?q=s:chroot&d:..20170324&x=t#t Additionally, workers may run in a completely different VM space (nullifying preload_app and any CoW savings) with the new worker_exec option: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-worker_exec https://bogomips.org/unicorn-public/?q=s:worker_exec&d:..20170324&x=t#t There are also several improvements to FreeBSD and OpenBSD support with the addition of these features. 34 changes since 5.2.0 (2016-10-31): Eric Wong (27): drop rb_str_set_len compatibility replacement TUNING: document THP caveat for Linux users tee_input: simplify condition for IO#write remove response_start_sent http_request: freeze constant strings passed IO#write Revert "remove response_start_sent" t/t0012-reload-empty-config.sh: access ivars directly if needed t0011-active-unix-socket.sh: fix race condition in test new test for check_client_connection revert signature change to HttpServer#process_client support "struct tcp_info" on non-Linux and Ruby 2.2+ unicorn_http: reduce rb_global_variable calls oob_gc: rely on opt_aref_with optimization on Ruby 2.2+ http_request: reduce insn size for check_client_connection freebsd: avoid EINVAL when setting accept filter test-lib: expr(1) portability fix tests: keep disabled tests defined test_exec: SO_KEEPALIVE value only needs to be true doc: fix links to raindrops project http_request: support proposed Raindrops::TCP states on non-Linux ISSUES: expand on mail archive info + subscription disclaimer test_ccc: use a pipe to synchronize test doc: remove private email support address input: update documentation and hide internals. http_server: initialize @pid ivar gemspec: remove olddoc from build dependency doc: add version annotations for new features Jeremy Evans (6): Add after_worker_exit configuration option Fix code example in after_worker_exit documentation Add support for chroot to Worker#user Add after_worker_ready configuration option Add worker_exec configuration option Don't pass a block for fork when forking workers Simon Eskildsen (1): check_client_connection: use tcp state on linux -- 5.3.0 in a week, maybe?
Simon Eskildsen <simon.eskildsen@shopify.com> wrote:
> Looks like Puma encountered some issues with some Linux distro's
> kernels not supporting this. That's crazy..
>
> https://github.com/puma/puma/issues/1241
No, it's not a Linux kernel problem.
That looks like a Unix socket they're using, but they only use
TCPServer.for_fd in their import_from_env function, and
inherit_unix_listener leaves TCPServer objects as-is.
TCPServer.for_fd won't barf if given a Unix socket.
In other words, this is fine:
u = UNIXServer.new '/tmp/foo'
t = TCPServer.for_fd(u.fileno)
t.accept
Feel free to forward this to them, I do not use non-Free
messaging systems (and I hate centralized ones).
Looks like Puma encountered some issues with some Linux distro's kernels not supporting this. That's crazy.. https://github.com/puma/puma/issues/1241
Awesome! Thanks for the update. I'll merge ccc-tcp-v3 into master soon and push out 5.3.0-rc1 (Maybe there's need to quote at all, every message is archived forever in several public places: https://bogomips.org/unicorn-public/ https://www.mail-archive.com/unicorn-public@bogomips.org maybe some others...)
I've put ccc-tcp-v3 in production today--it appears to be working as
expected, still rejecting at the exact same rate as before (100-200
per second for us during steady-state).
On Wed, Mar 8, 2017 at 7:06 AM, Simon Eskildsen
<simon.eskildsen@shopify.com> wrote:
> Patch-set looks great Eric, thanks!
>
> I'm hoping to test this in production later this week or next, but
> we're a year behind on Unicorn (ugh), so will need to carefully roll
> that out.
>
> On Tue, Mar 7, 2017 at 7:26 PM, Eric Wong <e@80x24.org> wrote:
>> Eric Wong <e@80x24.org> wrote:
>>> Simon Eskildsen <simon.eskildsen@shopify.com> wrote:
>>> > @@ -28,6 +29,7 @@ class Unicorn::HttpParser
>>> > # Drop these frozen strings when Ruby 2.2 becomes more prevalent,
>>> > # 2.2+ optimizes hash assignments when used with literal string keys
>>> > HTTP_RESPONSE_START = [ 'HTTP', '/1.1 ']
>>> > + EMPTY_ARRAY = [].freeze
>>>
>>> (minor) this was made before commit e06b699683738f22
>>> ("http_request: freeze constant strings passed IO#write")
>>> but I've trivially fixed it up, locally.
>>
>> And I actually screwed it up, pushed out ccc-tcp-v2 branch
>> with that fix squashed in :x
>>
>> Writing tests, now...
Patch-set looks great Eric, thanks!
I'm hoping to test this in production later this week or next, but
we're a year behind on Unicorn (ugh), so will need to carefully roll
that out.
On Tue, Mar 7, 2017 at 7:26 PM, Eric Wong <e@80x24.org> wrote:
> Eric Wong <e@80x24.org> wrote:
>> Simon Eskildsen <simon.eskildsen@shopify.com> wrote:
>> > @@ -28,6 +29,7 @@ class Unicorn::HttpParser
>> > # Drop these frozen strings when Ruby 2.2 becomes more prevalent,
>> > # 2.2+ optimizes hash assignments when used with literal string keys
>> > HTTP_RESPONSE_START = [ 'HTTP', '/1.1 ']
>> > + EMPTY_ARRAY = [].freeze
>>
>> (minor) this was made before commit e06b699683738f22
>> ("http_request: freeze constant strings passed IO#write")
>> but I've trivially fixed it up, locally.
>
> And I actually screwed it up, pushed out ccc-tcp-v2 branch
> with that fix squashed in :x
>
> Writing tests, now...
This looks great Eric. Thanks for taking this to the finish line!
On Wed, Mar 8, 2017 at 1:02 AM, Eric Wong <e@80x24.org> wrote:
> This series goes on top of Simon's excellent work to get
> a TCP_INFO-based check_client_connection working under Linux.
>
> First off, add a test extracted from one of Simon's messages;
> then revert the signature changes to existing methods to
> avoid breaking tmm1/gctools, and finally add a more portable
> fallback for Ruby 2.2+ users (tested on FreeBSD).
>
> Further work will improve portability of raindrops for FreeBSD
> (and maybe other *BSDs incidentally, too). That will be sent to
> raindrops-public@bogomips => https://bogomips.org/raindrops-public/
>
> Eric Wong (3):
> new test for check_client_connection
> revert signature change to HttpServer#process_client
> support "struct tcp_info" on non-Linux and Ruby 2.2+
>
> lib/unicorn/http_request.rb | 42 +++++++++++++++++++----
> lib/unicorn/http_server.rb | 6 ++--
> lib/unicorn/oob_gc.rb | 4 +--
> lib/unicorn/socket_helper.rb | 16 +++++++--
> test/unit/test_ccc.rb | 81 ++++++++++++++++++++++++++++++++++++++++++++
> test/unit/test_request.rb | 28 +++++++--------
> 6 files changed, 150 insertions(+), 27 deletions(-)
> create mode 100644 test/unit/test_ccc.rb
>
> Also pushed to the ccc-tcp-v3 branch:
>
> The following changes since commit 73e1ce827faad59bfcaff0bc758c8255a5e4f747:
>
> t0011-active-unix-socket.sh: fix race condition in test (2017-03-01 00:24:04 +0000)
>
> are available in the git repository at:
>
> git://bogomips.org/unicorn ccc-tcp-v3
>
> for you to fetch changes up to 873218e317773462be2a72556d26dc4a723cc7be:
>
> support "struct tcp_info" on non-Linux and Ruby 2.2+ (2017-03-08 05:39:55 +0000)
>
> ----------------------------------------------------------------
> Eric Wong (3):
> new test for check_client_connection
> revert signature change to HttpServer#process_client
> support "struct tcp_info" on non-Linux and Ruby 2.2+
>
> Simon Eskildsen (1):
> check_client_connection: use tcp state on linux
>
> lib/unicorn/http_request.rb | 73 ++++++++++++++++++++++++++++++++++++---
> lib/unicorn/socket_helper.rb | 16 +++++++--
> test/unit/test_ccc.rb | 81 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 163 insertions(+), 7 deletions(-)
> create mode 100644 test/unit/test_ccc.rb
>
> --
> EW
This series goes on top of Simon's excellent work to get a TCP_INFO-based check_client_connection working under Linux. First off, add a test extracted from one of Simon's messages; then revert the signature changes to existing methods to avoid breaking tmm1/gctools, and finally add a more portable fallback for Ruby 2.2+ users (tested on FreeBSD). Further work will improve portability of raindrops for FreeBSD (and maybe other *BSDs incidentally, too). That will be sent to raindrops-public@bogomips => https://bogomips.org/raindrops-public/ Eric Wong (3): new test for check_client_connection revert signature change to HttpServer#process_client support "struct tcp_info" on non-Linux and Ruby 2.2+ lib/unicorn/http_request.rb | 42 +++++++++++++++++++---- lib/unicorn/http_server.rb | 6 ++-- lib/unicorn/oob_gc.rb | 4 +-- lib/unicorn/socket_helper.rb | 16 +++++++-- test/unit/test_ccc.rb | 81 ++++++++++++++++++++++++++++++++++++++++++++ test/unit/test_request.rb | 28 +++++++-------- 6 files changed, 150 insertions(+), 27 deletions(-) create mode 100644 test/unit/test_ccc.rb Also pushed to the ccc-tcp-v3 branch: The following changes since commit 73e1ce827faad59bfcaff0bc758c8255a5e4f747: t0011-active-unix-socket.sh: fix race condition in test (2017-03-01 00:24:04 +0000) are available in the git repository at: git://bogomips.org/unicorn ccc-tcp-v3 for you to fetch changes up to 873218e317773462be2a72556d26dc4a723cc7be: support "struct tcp_info" on non-Linux and Ruby 2.2+ (2017-03-08 05:39:55 +0000) ---------------------------------------------------------------- Eric Wong (3): new test for check_client_connection revert signature change to HttpServer#process_client support "struct tcp_info" on non-Linux and Ruby 2.2+ Simon Eskildsen (1): check_client_connection: use tcp state on linux lib/unicorn/http_request.rb | 73 ++++++++++++++++++++++++++++++++++++--- lib/unicorn/socket_helper.rb | 16 +++++++-- test/unit/test_ccc.rb | 81 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 test/unit/test_ccc.rb -- EW
Eric Wong <e@80x24.org> wrote:
> Simon Eskildsen <simon.eskildsen@shopify.com> wrote:
> > @@ -28,6 +29,7 @@ class Unicorn::HttpParser
> > # Drop these frozen strings when Ruby 2.2 becomes more prevalent,
> > # 2.2+ optimizes hash assignments when used with literal string keys
> > HTTP_RESPONSE_START = [ 'HTTP', '/1.1 ']
> > + EMPTY_ARRAY = [].freeze
>
> (minor) this was made before commit e06b699683738f22
> ("http_request: freeze constant strings passed IO#write")
> but I've trivially fixed it up, locally.
And I actually screwed it up, pushed out ccc-tcp-v2 branch
with that fix squashed in :x
Writing tests, now...
Simon Eskildsen <simon.eskildsen@shopify.com> wrote: > Here's another update Eric! Thanks, a few teeny issues fixed up locally (but commented inline, below). However, there's a bigger problem which I'm Cc:-ing Aman about for tmm1/gctools changing process_client in the internal API. I won't burden him maintaining extra versions, nor will I force users to use a certain version of unicorn or gctools to match. Aman: for reference, relevant messages in the archives: https://bogomips.org/unicorn-public/?q=d:20170222-+check_client_connection&x=t (TL;DR: I don't expect Aman will have to do anything, just keeping him in the loop) > +++ b/lib/unicorn/http_server.rb > @@ -558,8 +558,8 @@ def e100_response_write(client, env) > > # once a client is accepted, it is processed in its entirety here > # in 3 easy steps: read request, call app, write app response > - def process_client(client) > - status, headers, body = @app.call(env = @request.read(client)) > + def process_client(client, listener) > + status, headers, body = @app.call(env = @request.read(client, listener)) I can squash this fix in, locally: diff --git a/lib/unicorn/oob_gc.rb b/lib/unicorn/oob_gc.rb index 5572e59..74a1d51 100644 --- a/lib/unicorn/oob_gc.rb +++ b/lib/unicorn/oob_gc.rb @@ -67,8 +67,8 @@ def self.new(app, interval = 5, path = %r{\A/}) #:stopdoc: PATH_INFO = "PATH_INFO" - def process_client(client) - super(client) # Unicorn::HttpServer#process_client + def process_client(client, listener) + super(client, listener) # Unicorn::HttpServer#process_client if OOBGC_PATH =~ OOBGC_ENV[PATH_INFO] && ((@@nr -= 1) <= 0) @@nr = OOBGC_INTERVAL OOBGC_ENV.clear However, https://github.com/tmm1/gctools also depends on this undocumented internal API of ours; and I will not consider breaking it for release... Pushed to the "ccc-tcp" branch @ git://bogomips.org/unicorn (commit beaee769c6553bf4e0260be2507b8235f0aa764f) > begin > return if @request.hijacked? > @@ -655,7 +655,7 @@ def worker_loop(worker) > # Unicorn::Worker#kgio_tryaccept is not like accept(2) at all, > # but that will return false > if client = sock.kgio_tryaccept > - process_client(client) > + process_client(client, sock) > nr += 1 > worker.tick = time_now.to_i > end > @@ -28,6 +29,7 @@ class Unicorn::HttpParser > # Drop these frozen strings when Ruby 2.2 becomes more prevalent, > # 2.2+ optimizes hash assignments when used with literal string keys > HTTP_RESPONSE_START = [ 'HTTP', '/1.1 '] > + EMPTY_ARRAY = [].freeze (minor) this was made before commit e06b699683738f22 ("http_request: freeze constant strings passed IO#write") but I've trivially fixed it up, locally. > + def check_client_connection(socket, listener) # :nodoc: > + if Kgio::TCPServer === listener > + @@tcp_info ||= Raindrops::TCP_Info.new(socket) > + @@tcp_info.get!(socket) > + raise Errno::EPIPE, "client closed connection".freeze, > EMPTY_ARRAY if closed_state?(@@tcp_info.state) (minor) I needed to wrap that line since I use gigantic fonts (fixed up locally)
Here's another update Eric! * Use a frozen empty array and a class variable for TCP_Info to avoid garbage. As far as I can tell, this shouldn't result in any garbage on any requests (other than on the first request). * Pass listener socket to #read to only check the client connection on a TCP server. * Short circuit CLOSE_WAIT after ESTABLISHED since in my testing it's the most common state after ESTABLISHED, it makes the numbers un-ordered, though. But comment should make it OK. * Definition of of `check_client_connection` based on whether Raindrops::TCP_Info is defined, instead of the class variable approach. * Changed the unit tests to pass a `nil` listener. Tested on our staging environment, and still works like a dream. I should note that I got the idea between this patch into Puma as well! https://github.com/puma/puma/pull/1227 --- lib/unicorn/http_request.rb | 44 ++++++++++++++++++++++++++++++++++++++------ lib/unicorn/http_server.rb | 6 +++--- test/unit/test_request.rb | 28 ++++++++++++++-------------- 3 files changed, 55 insertions(+), 23 deletions(-) diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb index 0c1f9bb..21a99ef 100644 --- a/lib/unicorn/http_request.rb +++ b/lib/unicorn/http_request.rb @@ -2,6 +2,7 @@ # :enddoc: # no stable API here require 'unicorn_http' +require 'raindrops' # TODO: remove redundant names Unicorn.const_set(:HttpRequest, Unicorn::HttpParser) @@ -28,6 +29,7 @@ class Unicorn::HttpParser # Drop these frozen strings when Ruby 2.2 becomes more prevalent, # 2.2+ optimizes hash assignments when used with literal string keys HTTP_RESPONSE_START = [ 'HTTP', '/1.1 '] + EMPTY_ARRAY = [].freeze @@input_class = Unicorn::TeeInput @@check_client_connection = false @@ -62,7 +64,7 @@ def self.check_client_connection=(bool) # returns an environment hash suitable for Rack if successful # This does minimal exception trapping and it is up to the caller # to handle any socket errors (e.g. user aborted upload). - def read(socket) + def read(socket, listener) clear e = env @@ -83,11 +85,7 @@ def read(socket) false until add_parse(socket.kgio_read!(16384)) end - # detect if the socket is valid by writing a partial response: - if @@check_client_connection && headers? - self.response_start_sent = true - HTTP_RESPONSE_START.each { |c| socket.write(c) } - end + check_client_connection(socket, listener) if @@check_client_connection e['rack.input'] = 0 == content_length ? NULL_IO : @@input_class.new(socket, self) @@ -108,4 +106,38 @@ def call def hijacked? env.include?('rack.hijack_io'.freeze) end + + if defined?(Raindrops::TCP_Info) + def check_client_connection(socket, listener) # :nodoc: + if Kgio::TCPServer === listener + @@tcp_info ||= Raindrops::TCP_Info.new(socket) + @@tcp_info.get!(socket) + raise Errno::EPIPE, "client closed connection".freeze, EMPTY_ARRAY if closed_state?(@@tcp_info.state) + else + write_http_header(socket) + end + end + + def closed_state?(state) # :nodoc: + case state + when 1 # ESTABLISHED + false + when 8, 6, 7, 9, 11 # CLOSE_WAIT, TIME_WAIT, CLOSE, LAST_ACK, CLOSING + true + else + false + end + end + else + def check_client_connection(socket, listener) # :nodoc: + write_http_header(socket) + end + end + + def write_http_header(socket) # :nodoc: + if headers? + self.response_start_sent = true + HTTP_RESPONSE_START.each { |c| socket.write(c) } + end + end end diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb index 35bd100..4190641 100644 --- a/lib/unicorn/http_server.rb +++ b/lib/unicorn/http_server.rb @@ -558,8 +558,8 @@ def e100_response_write(client, env) # once a client is accepted, it is processed in its entirety here # in 3 easy steps: read request, call app, write app response - def process_client(client) - status, headers, body = @app.call(env = @request.read(client)) + def process_client(client, listener) + status, headers, body = @app.call(env = @request.read(client, listener)) begin return if @request.hijacked? @@ -655,7 +655,7 @@ def worker_loop(worker) # Unicorn::Worker#kgio_tryaccept is not like accept(2) at all, # but that will return false if client = sock.kgio_tryaccept - process_client(client) + process_client(client, sock) nr += 1 worker.tick = time_now.to_i end diff --git a/test/unit/test_request.rb b/test/unit/test_request.rb index f0ccaf7..dbe8af7 100644 --- a/test/unit/test_request.rb +++ b/test/unit/test_request.rb @@ -30,7 +30,7 @@ def setup def test_options client = MockRequest.new("OPTIONS * HTTP/1.1\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal '', env['REQUEST_PATH'] assert_equal '', env['PATH_INFO'] assert_equal '*', env['REQUEST_URI'] @@ -40,7 +40,7 @@ def test_options def test_absolute_uri_with_query client = MockRequest.new("GET http://e:3/x?y=z HTTP/1.1\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal '/x', env['REQUEST_PATH'] assert_equal '/x', env['PATH_INFO'] assert_equal 'y=z', env['QUERY_STRING'] @@ -50,7 +50,7 @@ def test_absolute_uri_with_query def test_absolute_uri_with_fragment client = MockRequest.new("GET http://e:3/x#frag HTTP/1.1\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal '/x', env['REQUEST_PATH'] assert_equal '/x', env['PATH_INFO'] assert_equal '', env['QUERY_STRING'] @@ -61,7 +61,7 @@ def test_absolute_uri_with_fragment def test_absolute_uri_with_query_and_fragment client = MockRequest.new("GET http://e:3/x?a=b#frag HTTP/1.1\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal '/x', env['REQUEST_PATH'] assert_equal '/x', env['PATH_INFO'] assert_equal 'a=b', env['QUERY_STRING'] @@ -73,7 +73,7 @@ def test_absolute_uri_unsupported_schemes %w(ssh+http://e/ ftp://e/x http+ssh://e/x).each do |abs_uri| client = MockRequest.new("GET #{abs_uri} HTTP/1.1\r\n" \ "Host: foo\r\n\r\n") - assert_raises(HttpParserError) { @request.read(client) } + assert_raises(HttpParserError) { @request.read(client, nil) } end end @@ -81,7 +81,7 @@ def test_x_forwarded_proto_https client = MockRequest.new("GET / HTTP/1.1\r\n" \ "X-Forwarded-Proto: https\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal "https", env['rack.url_scheme'] res = @lint.call(env) end @@ -90,7 +90,7 @@ def test_x_forwarded_proto_http client = MockRequest.new("GET / HTTP/1.1\r\n" \ "X-Forwarded-Proto: http\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal "http", env['rack.url_scheme'] res = @lint.call(env) end @@ -99,14 +99,14 @@ def test_x_forwarded_proto_invalid client = MockRequest.new("GET / HTTP/1.1\r\n" \ "X-Forwarded-Proto: ftp\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal "http", env['rack.url_scheme'] res = @lint.call(env) end def test_rack_lint_get client = MockRequest.new("GET / HTTP/1.1\r\nHost: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal "http", env['rack.url_scheme'] assert_equal '127.0.0.1', env['REMOTE_ADDR'] res = @lint.call(env) @@ -114,7 +114,7 @@ def test_rack_lint_get def test_no_content_stringio client = MockRequest.new("GET / HTTP/1.1\r\nHost: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal StringIO, env['rack.input'].class end @@ -122,7 +122,7 @@ def test_zero_content_stringio client = MockRequest.new("PUT / HTTP/1.1\r\n" \ "Content-Length: 0\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal StringIO, env['rack.input'].class end @@ -130,7 +130,7 @@ def test_real_content_not_stringio client = MockRequest.new("PUT / HTTP/1.1\r\n" \ "Content-Length: 1\r\n" \ "Host: foo\r\n\r\n") - env = @request.read(client) + env = @request.read(client, nil) assert_equal Unicorn::TeeInput, env['rack.input'].class end @@ -141,7 +141,7 @@ def test_rack_lint_put "Content-Length: 5\r\n" \ "\r\n" \ "abcde") - env = @request.read(client) + env = @request.read(client, nil) assert ! env.include?(:http_body) res = @lint.call(env) end @@ -167,7 +167,7 @@ def client.kgio_read!(*args) "\r\n") count.times { assert_equal bs, client.syswrite(buf) } assert_equal 0, client.sysseek(0) - env = @request.read(client) + env = @request.read(client, nil) assert ! env.include?(:http_body) assert_equal length, env['rack.input'].size count.times { -- 2.11.0 On Tue, Feb 28, 2017 at 10:18 PM, Eric Wong <e@80x24.org> wrote: >> Simon Eskildsen <simon.eskildsen@shopify.com> wrote: >> > + tcp_info = Raindrops::TCP_Info.new(socket) >> > + raise Errno::EPIPE, "client closed connection".freeze, [] if >> > closed_state?(tcp_info.state) > > Also, I guess if you're using this frequently, it might make > sense to keep the tcp_info object around and recycle it > using the Raindrops::TCP_Info#get! method. > > #get! has always been supported in raindrops, but I just noticed > RDoc wasn't picking it up properly :x > > Anyways I've fixed the documentation over on the raindrops list > > https://bogomips.org/raindrops-public/20170301025541.26183-1-e@80x24.org/ > ("[PATCH] ext: fix documentation for C ext-defined classes") > > https://bogomips.org/raindrops-public/20170301025546.26233-1-e@80x24.org/ > ("[PATCH] TCP_Info: custom documentation for #get!") > > ... and updated the RDoc on https://bogomips.org/raindrops/ > > Heck, I wonder if it even makes sense to reuse a frozen empty > Array when raising the exception...
> Simon Eskildsen <simon.eskildsen@shopify.com> wrote: > > + tcp_info = Raindrops::TCP_Info.new(socket) > > + raise Errno::EPIPE, "client closed connection".freeze, [] if > > closed_state?(tcp_info.state) Also, I guess if you're using this frequently, it might make sense to keep the tcp_info object around and recycle it using the Raindrops::TCP_Info#get! method. #get! has always been supported in raindrops, but I just noticed RDoc wasn't picking it up properly :x Anyways I've fixed the documentation over on the raindrops list https://bogomips.org/raindrops-public/20170301025541.26183-1-e@80x24.org/ ("[PATCH] ext: fix documentation for C ext-defined classes") https://bogomips.org/raindrops-public/20170301025546.26233-1-e@80x24.org/ ("[PATCH] TCP_Info: custom documentation for #get!") ... and updated the RDoc on https://bogomips.org/raindrops/ Heck, I wonder if it even makes sense to reuse a frozen empty Array when raising the exception...
Simon Eskildsen <simon.eskildsen@shopify.com> wrote: <snip> > I would assume you would see TIME_WAIT and CLOSE. LAST_ACK_CLOSING it > seems pretty unlikely to hit, but not impossible. As with CLOSING, > I've included LAST_ACK_CLOSING for completeness. Did you mean "LAST_ACK, and CLOSING"? (not joined by underscore) Anyways, thanks for testing and adding > <e@80x24.org> wrote: > > Yep, we need to account for the UNIX socket case. I forget if > > kgio even makes them different... > > I read the implementation and verified by dumping the class when > testing on some test boxes. You are right—it's a simple Kgio::Socket > object, not differentiating between Kgio::TCPSocket and > Kgio::UnixSocket at the class level. Kgio only does this if they're > explicitly passed to override the class returned from #try_accept. > Unicorn doesn't do this. > > I've tried to find a way to determine the socket domain (INET vs. > UNIX) on the socket object, but neither Ruby's Socket class nor Kgio > seems to expose this. I'm not entirely sure what the simplest way to > do this check would be. We could have the accept loop pass the correct > class to #try_accept based on the listening socket that came back from > #accept. If we passed the listening socket to #read after accept, we'd > know.. but I don't like that the request knows about the listener > either. Alternatively, we could expose the socket domain in Kgio, but > that'll be problematic in the near-ish future as you've mentioned > wanting to move away from Kgio as Ruby's IO library is at parity as > per Ruby 2.4. > > What do you suggest pursuing here to check whether the client socket > is a TCP socket? I think passing the listening socket is the best way to go about detecting whether a socket is INET or UNIX, for now. You're right about kgio, I'd rather not make more changes to kgio but we will still need it to for Ruby <2.2.x users. And #read is an overloaded name, feel free to change it :) > Below is a patch addressing the other concerns. I had to include > require raindrops so the `defined?` check would do the right thing, as > the only other file that requires Raindrops is the worker one which is > loaded after http_request. I can change the load-order or require > raindrops in lib/unicorn.rb if you prefer. The require is fine. However we do not need a class variable, below... > # TODO: remove redundant names > Unicorn.const_set(:HttpRequest, Unicorn::HttpParser) > @@ -29,6 +30,7 @@ class Unicorn::HttpParser > # 2.2+ optimizes hash assignments when used with literal string keys > HTTP_RESPONSE_START = [ 'HTTP', '/1.1 '] > @@input_class = Unicorn::TeeInput > + @@raindrops_tcp_info_defined = defined?(Raindrops::TCP_Info) I prefer we avoid adding this cvar, instead... > @@check_client_connection = false > > def self.input_class > @@ -83,11 +85,7 @@ def read(socket) > false until add_parse(socket.kgio_read!(16384)) > end > > - # detect if the socket is valid by writing a partial response: > - if @@check_client_connection && headers? > - self.response_start_sent = true > - HTTP_RESPONSE_START.each { |c| socket.write(c) } > - end > + check_client_connection(socket) if @@check_client_connection > > e['rack.input'] = 0 == content_length ? > NULL_IO : @@input_class.new(socket, self) > @@ -108,4 +106,27 @@ def call > def hijacked? > env.include?('rack.hijack_io'.freeze) > end ... we can have different methods defined: if defined?(Raindrops::TCP_Info) # Linux, maybe FreeBSD def check_client_connection(client, listener) # :nodoc: ... end else # portable version def check_client_connection(client, listener) # :nodoc: ... end end And eliminate the class variable entirely. > + > + private I prefer to avoid marking methods as 'private' to ease any ad-hoc unit testing which may come up. Instead, rely on :nodoc: directives to discourage people from depending on it. Thanks. > + def check_client_connection(socket) > + if @@raindrops_tcp_info_defined > + tcp_info = Raindrops::TCP_Info.new(socket) > + raise Errno::EPIPE, "client closed connection".freeze, [] if > closed_state?(tcp_info.state) > + elsif headers? > + self.response_start_sent = true > + HTTP_RESPONSE_START.each { |c| socket.write(c) } > + end > + end > + > + def closed_state?(state) > + case state > + when 1 # ESTABLISHED > + false > + when 6, 7, 8, 9, 11 # TIME_WAIT, CLOSE, CLOSE_WAIT, LAST_ACK, CLOSING > + true > + else > + false > + end > + end > end closed_state? looks good to me, good call on short-circuiting the common case of ESTABLISHED!
> I prefer we use a hash or case statement. Both allow more > optimization in the YARV VM of CRuby (opt_aref and > opt_case_dispatch in insns.def). case _might_ be a little > faster if there's no constant lookup overhead, but > a microbench or dumping the bytecode will be necessary > to be sure :) > > A hash or a case can also help portability-wise in case > we hit a system where these numbers are non-sequential; > or if we forgot something. Good point. I double checked all the states on Linux and found that we were missing TCP_CLOSING [1] [2]. This is a state where the other side is closed, and you have buffered data on your side. It doesn't seem like this would ever happen in Unicorn, but I think we should include it for completeness. This also means the range becomes non-sequential. I looked at Illumus (solaris-derived) [3] and BSD [4] and for the TCP states we're interested in it also appears to have a non-continues range. My co-worker, Kir Shatrov, benchmarked a bunch of approaches to the state check and found that case is a good solution [5]. Due to the realness of non-sequential states in common operating systems, I think case is the way to go here as you suggested. I've made sure to short-circuit the common-case of TCP_ESTABLISHED. I've only seen CLOSE_WAIT in testing, but in the wild-life of large production scale I would assume you would see TIME_WAIT and CLOSE. LAST_ACK_CLOSING it seems pretty unlikely to hit, but not impossible. As with CLOSING, I've included LAST_ACK_CLOSING for completeness. [1] https://github.com/torvalds/linux/blob/5924bbecd0267d87c24110cbe2041b5075173a25/include/net/tcp_states.h#L27 [2] https://github.com/torvalds/linux/blob/ca78d3173cff3503bcd15723b049757f75762d15/net/ipv4/tcp.c#L228 [3] https://github.com/freebsd/freebsd/blob/386ddae58459341ec567604707805814a2128a57/sys/netinet/tcp_fsm.h [4] https://github.com/illumos/illumos-gate/blob/f7877f5d39900cfd8b20dd673e5ccc1ef7cc7447/usr/src/uts/common/netinet/tcp_fsm.h [5] https://gist.github.com/kirs/11ba4ce84c08188c9f7eba9c639616a5 > Yep, we need to account for the UNIX socket case. I forget if > kgio even makes them different... I read the implementation and verified by dumping the class when testing on some test boxes. You are right—it's a simple Kgio::Socket object, not differentiating between Kgio::TCPSocket and Kgio::UnixSocket at the class level. Kgio only does this if they're explicitly passed to override the class returned from #try_accept. Unicorn doesn't do this. I've tried to find a way to determine the socket domain (INET vs. UNIX) on the socket object, but neither Ruby's Socket class nor Kgio seems to expose this. I'm not entirely sure what the simplest way to do this check would be. We could have the accept loop pass the correct class to #try_accept based on the listening socket that came back from #accept. If we passed the listening socket to #read after accept, we'd know.. but I don't like that the request knows about the listener either. Alternatively, we could expose the socket domain in Kgio, but that'll be problematic in the near-ish future as you've mentioned wanting to move away from Kgio as Ruby's IO library is at parity as per Ruby 2.4. What do you suggest pursuing here to check whether the client socket is a TCP socket? Below is a patch addressing the other concerns. I had to include require raindrops so the `defined?` check would do the right thing, as the only other file that requires Raindrops is the worker one which is loaded after http_request. I can change the load-order or require raindrops in lib/unicorn.rb if you prefer. Missing is the socket type check. Thanks for your feedback! --- lib/unicorn/http_request.rb | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb index 0c1f9bb..eedccac 100644 --- a/lib/unicorn/http_request.rb +++ b/lib/unicorn/http_request.rb @@ -2,6 +2,7 @@ # :enddoc: # no stable API here require 'unicorn_http' +require 'raindrops' # TODO: remove redundant names Unicorn.const_set(:HttpRequest, Unicorn::HttpParser) @@ -29,6 +30,7 @@ class Unicorn::HttpParser # 2.2+ optimizes hash assignments when used with literal string keys HTTP_RESPONSE_START = [ 'HTTP', '/1.1 '] @@input_class = Unicorn::TeeInput + @@raindrops_tcp_info_defined = defined?(Raindrops::TCP_Info) @@check_client_connection = false def self.input_class @@ -83,11 +85,7 @@ def read(socket) false until add_parse(socket.kgio_read!(16384)) end - # detect if the socket is valid by writing a partial response: - if @@check_client_connection && headers? - self.response_start_sent = true - HTTP_RESPONSE_START.each { |c| socket.write(c) } - end + check_client_connection(socket) if @@check_client_connection e['rack.input'] = 0 == content_length ? NULL_IO : @@input_class.new(socket, self) @@ -108,4 +106,27 @@ def call def hijacked? env.include?('rack.hijack_io'.freeze) end + + private + + def check_client_connection(socket) + if @@raindrops_tcp_info_defined + tcp_info = Raindrops::TCP_Info.new(socket) + raise Errno::EPIPE, "client closed connection".freeze, [] if closed_state?(tcp_info.state) + elsif headers? + self.response_start_sent = true + HTTP_RESPONSE_START.each { |c| socket.write(c) } + end + end + + def closed_state?(state) + case state + when 1 # ESTABLISHED + false + when 6, 7, 8, 9, 11 # TIME_WAIT, CLOSE, CLOSE_WAIT, LAST_ACK, CLOSING + true + else + false + end + end end -- 2.11.0
Simon Eskildsen <simon.eskildsen@shopify.com> wrote: > On Sat, Feb 25, 2017 at 9:03 AM, Simon Eskildsen > <simon.eskildsen@shopify.com> wrote: > > The implementation of the check_client_connection causing an early write is > > ineffective when not performed on loopback. In my testing, when on non-loopback, > > such as another host, we only see a 10-20% rejection rate with TCP_NODELAY of > > clients that are closed. This means 90-80% of responses in this case are > > rendered in vain. > > > > This patch uses getosockopt(2) with TCP_INFO on Linux to read the socket state. > > If the socket is in a state where it doesn't take a response, we'll abort the > > request with a similar error as to what write(2) would give us on a closed > > socket in case of an error. > > > > In my testing, this has a 100% rejection rate. This testing was conducted with > > the following script: Good to know! > > diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb > > index 0c1f9bb..b4c95fc 100644 > > --- a/lib/unicorn/http_request.rb > > +++ b/lib/unicorn/http_request.rb > > @@ -31,6 +31,9 @@ class Unicorn::HttpParser > > @@input_class = Unicorn::TeeInput > > @@check_client_connection = false > > > > + # TCP_TIME_WAIT: 6, TCP_CLOSE: 7, TCP_CLOSE_WAIT: 8, TCP_LAST_ACK: 9 > > + IGNORED_CHECK_CLIENT_SOCKET_STATES = (6..9) Thanks for documenting the numbers. I prefer we use a hash or case statement. Both allow more optimization in the YARV VM of CRuby (opt_aref and opt_case_dispatch in insns.def). case _might_ be a little faster if there's no constant lookup overhead, but a microbench or dumping the bytecode will be necessary to be sure :) A hash or a case can also help portability-wise in case we hit a system where these numbers are non-sequential; or if we forgot something. > > - # detect if the socket is valid by writing a partial response: > > - if @@check_client_connection && headers? > > - self.response_start_sent = true > > - HTTP_RESPONSE_START.each { |c| socket.write(c) } > > + # detect if the socket is valid by checking client connection. > > + if @@check_client_connection We can probably split everything inside this if to a new method, this avoid penalizing people who don't use this feature. Using check_client_connection will already have a high cost, since it requires at least one extra syscall. > > + if defined?(Raindrops::TCP_Info) ...because defined? only needs to be checked once for the lifetime of the process; we can define different methods at load time to avoid the check for each and every request. > > + tcp_info = Raindrops::TCP_Info.new(socket) > > + if IGNORED_CHECK_CLIENT_SOCKET_STATES.cover?(tcp_info.state) > > + socket.close Right, no need to socket.close, here; handle_error does it. > > + raise Errno::EPIPE Since we don't print out the backtrace in handle_error, we can raise without a backtrace to avoid excess garbage. > > + end > > + elsif headers? > > + self.response_start_sent = true > > + HTTP_RESPONSE_START.each { |c| socket.write(c) } > > + end > > end > I left out a TCPSocket check, we'll need a `socket.is_a?(TCPSocket)` > in there. I'll wait with sending an updated patch in case you have > other comments. I'm also not entirely sure we need the `socket.close`. > What do you think? Yep, we need to account for the UNIX socket case. I forget if kgio even makes them different... Anyways I won't be online much for a few days, and it's the weekend, so no rush :) Thanks.
I left out a TCPSocket check, we'll need a `socket.is_a?(TCPSocket)`
in there. I'll wait with sending an updated patch in case you have
other comments. I'm also not entirely sure we need the `socket.close`.
What do you think?
On Sat, Feb 25, 2017 at 9:03 AM, Simon Eskildsen
<simon.eskildsen@shopify.com> wrote:
> The implementation of the check_client_connection causing an early write is
> ineffective when not performed on loopback. In my testing, when on non-loopback,
> such as another host, we only see a 10-20% rejection rate with TCP_NODELAY of
> clients that are closed. This means 90-80% of responses in this case are
> rendered in vain.
>
> This patch uses getosockopt(2) with TCP_INFO on Linux to read the socket state.
> If the socket is in a state where it doesn't take a response, we'll abort the
> request with a similar error as to what write(2) would give us on a closed
> socket in case of an error.
>
> In my testing, this has a 100% rejection rate. This testing was conducted with
> the following script:
>
> 100.times do |i|
> client = TCPSocket.new("some-unicorn", 20_000)
> client.write("GET /collections/#{rand(10000)}
> HTTP/1.1\r\nHost:walrusser.myshopify.com\r\n\r\n")
> client.close
> end
> ---
> lib/unicorn/http_request.rb | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
> index 0c1f9bb..b4c95fc 100644
> --- a/lib/unicorn/http_request.rb
> +++ b/lib/unicorn/http_request.rb
> @@ -31,6 +31,9 @@ class Unicorn::HttpParser
> @@input_class = Unicorn::TeeInput
> @@check_client_connection = false
>
> + # TCP_TIME_WAIT: 6, TCP_CLOSE: 7, TCP_CLOSE_WAIT: 8, TCP_LAST_ACK: 9
> + IGNORED_CHECK_CLIENT_SOCKET_STATES = (6..9)
> +
> def self.input_class
> @@input_class
> end
> @@ -83,10 +86,18 @@ def read(socket)
> false until add_parse(socket.kgio_read!(16384))
> end
>
> - # detect if the socket is valid by writing a partial response:
> - if @@check_client_connection && headers?
> - self.response_start_sent = true
> - HTTP_RESPONSE_START.each { |c| socket.write(c) }
> + # detect if the socket is valid by checking client connection.
> + if @@check_client_connection
> + if defined?(Raindrops::TCP_Info)
> + tcp_info = Raindrops::TCP_Info.new(socket)
> + if IGNORED_CHECK_CLIENT_SOCKET_STATES.cover?(tcp_info.state)
> + socket.close
> + raise Errno::EPIPE
> + end
> + elsif headers?
> + self.response_start_sent = true
> + HTTP_RESPONSE_START.each { |c| socket.write(c) }
> + end
> end
>
> e['rack.input'] = 0 == content_length ?
> --
> 2.11.0
The implementation of the check_client_connection causing an early write is ineffective when not performed on loopback. In my testing, when on non-loopback, such as another host, we only see a 10-20% rejection rate with TCP_NODELAY of clients that are closed. This means 90-80% of responses in this case are rendered in vain. This patch uses getosockopt(2) with TCP_INFO on Linux to read the socket state. If the socket is in a state where it doesn't take a response, we'll abort the request with a similar error as to what write(2) would give us on a closed socket in case of an error. In my testing, this has a 100% rejection rate. This testing was conducted with the following script: 100.times do |i| client = TCPSocket.new("some-unicorn", 20_000) client.write("GET /collections/#{rand(10000)} HTTP/1.1\r\nHost:walrusser.myshopify.com\r\n\r\n") client.close end --- lib/unicorn/http_request.rb | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb index 0c1f9bb..b4c95fc 100644 --- a/lib/unicorn/http_request.rb +++ b/lib/unicorn/http_request.rb @@ -31,6 +31,9 @@ class Unicorn::HttpParser @@input_class = Unicorn::TeeInput @@check_client_connection = false + # TCP_TIME_WAIT: 6, TCP_CLOSE: 7, TCP_CLOSE_WAIT: 8, TCP_LAST_ACK: 9 + IGNORED_CHECK_CLIENT_SOCKET_STATES = (6..9) + def self.input_class @@input_class end @@ -83,10 +86,18 @@ def read(socket) false until add_parse(socket.kgio_read!(16384)) end - # detect if the socket is valid by writing a partial response: - if @@check_client_connection && headers? - self.response_start_sent = true - HTTP_RESPONSE_START.each { |c| socket.write(c) } + # detect if the socket is valid by checking client connection. + if @@check_client_connection + if defined?(Raindrops::TCP_Info) + tcp_info = Raindrops::TCP_Info.new(socket) + if IGNORED_CHECK_CLIENT_SOCKET_STATES.cover?(tcp_info.state) + socket.close + raise Errno::EPIPE + end + elsif headers? + self.response_start_sent = true + HTTP_RESPONSE_START.each { |c| socket.write(c) } + end end e['rack.input'] = 0 == content_length ? -- 2.11.0