* [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO
@ 2017-03-26 1:49 6% Dylan Thacker-Smith
2017-03-26 2:41 7% ` Eric Wong
0 siblings, 1 reply; 5+ results
From: Dylan Thacker-Smith @ 2017-03-26 1:49 UTC (permalink / raw)
To: unicorn-public
The ruby constant Socket::TCP_INFO is only defined if TCP_INFO is defined
in C, so we can just check for the presence of that ruby constant instead
of rescuing SocketError from the call to getsockopt.
---
lib/unicorn/http_request.rb | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index 6dc0aa7..8f1638a 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -29,7 +29,7 @@ class Unicorn::HttpParser
EMPTY_ARRAY = [].freeze
@@input_class = Unicorn::TeeInput
@@check_client_connection = false
- @@tcpi_inspect_ok = nil
+ @@tcpi_inspect_ok = Socket.const_defined?(:TCP_INFO)
def self.input_class
@@input_class
@@ -154,20 +154,10 @@ def closed_state?(state) # :nodoc:
# Not that efficient, but probably still better than doing unnecessary
# work after a client gives up.
def check_client_connection(socket) # :nodoc:
- if Unicorn::TCPClient === socket && @@tcpi_inspect_ok != false
- if @@tcpi_inspect_ok
- opt = socket.getsockopt(:IPPROTO_TCP, :TCP_INFO).inspect
- else
- @@tcpi_inspect_ok = true
- opt = begin
- socket.getsockopt(:IPPROTO_TCP, :TCP_INFO)
- rescue SocketError
- @@tcpi_inspect_ok = false
- return write_http_header(socket)
- end.inspect
- end
-
+ if Unicorn::TCPClient === socket && @@tcpi_inspect_ok
+ opt = socket.getsockopt(Socket::IPPROTO_TCP, Socket::TCP_INFO).inspect
if opt =~ /\bstate=(\S+)/
+ @@tcpi_inspect_ok = true
raise Errno::EPIPE, "client closed connection".freeze,
EMPTY_ARRAY if closed_state_str?($1)
else
--
2.10.0
^ permalink raw reply related [relevance 6%]
* Re: [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO
2017-03-26 1:49 6% [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO Dylan Thacker-Smith
@ 2017-03-26 2:41 7% ` Eric Wong
2017-03-26 2:52 7% ` Dylan Thacker-Smith
0 siblings, 1 reply; 5+ results
From: Eric Wong @ 2017-03-26 2:41 UTC (permalink / raw)
To: Dylan Thacker-Smith; +Cc: unicorn-public
Dylan Thacker-Smith <dylan.smith@shopify.com> wrote:
> The ruby constant Socket::TCP_INFO is only defined if TCP_INFO is defined
> in C, so we can just check for the presence of that ruby constant instead
> of rescuing SocketError from the call to getsockopt.
Good catch! I forget there's systems without TCP_INFO at all :x
> +++ b/lib/unicorn/http_request.rb
> @@ -29,7 +29,7 @@ class Unicorn::HttpParser
> EMPTY_ARRAY = [].freeze
> @@input_class = Unicorn::TeeInput
> @@check_client_connection = false
> - @@tcpi_inspect_ok = nil
> + @@tcpi_inspect_ok = Socket.const_defined?(:TCP_INFO)
>
> def self.input_class
> @@input_class
> @@ -154,20 +154,10 @@ def closed_state?(state) # :nodoc:
> # Not that efficient, but probably still better than doing unnecessary
> # work after a client gives up.
> def check_client_connection(socket) # :nodoc:
> - if Unicorn::TCPClient === socket && @@tcpi_inspect_ok != false
> - if @@tcpi_inspect_ok
> - opt = socket.getsockopt(:IPPROTO_TCP, :TCP_INFO).inspect
> - else
> - @@tcpi_inspect_ok = true
> - opt = begin
> - socket.getsockopt(:IPPROTO_TCP, :TCP_INFO)
> - rescue SocketError
> - @@tcpi_inspect_ok = false
> - return write_http_header(socket)
> - end.inspect
> - end
> -
> + if Unicorn::TCPClient === socket && @@tcpi_inspect_ok
> + opt = socket.getsockopt(Socket::IPPROTO_TCP, Socket::TCP_INFO).inspect
> if opt =~ /\bstate=(\S+)/
> + @@tcpi_inspect_ok = true
Is the last line to set "@@tcpi_inspect_ok = true" still
necessary? It was necessary in Jeremy's version, but I think
you can safely omit it, now.
> raise Errno::EPIPE, "client closed connection".freeze,
> EMPTY_ARRAY if closed_state_str?($1)
> else
Anyways, I'm inclined to apply your patch with the redundant
assignment removed (no need to resend). Thanks.
^ permalink raw reply [relevance 7%]
* Re: [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO
2017-03-26 2:41 7% ` Eric Wong
@ 2017-03-26 2:52 7% ` Dylan Thacker-Smith
2017-03-26 3:37 7% ` Eric Wong
0 siblings, 1 reply; 5+ results
From: Dylan Thacker-Smith @ 2017-03-26 2:52 UTC (permalink / raw)
To: Eric Wong; +Cc: unicorn-public
On Sat, Mar 25, 2017 at 10:41 PM, Eric Wong <e@80x24.org> wrote:
> Is the last line to set "@@tcpi_inspect_ok = true" still
> necessary? It was necessary in Jeremy's version, but I think
> you can safely omit it, now.
No it isn't needed, so you can remove it.
I had just started my patch before seeing his, so reverted his as part
of rebasing and this line was present before his patch.
^ permalink raw reply [relevance 7%]
* Re: [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO
2017-03-26 2:52 7% ` Dylan Thacker-Smith
@ 2017-03-26 3:37 7% ` Eric Wong
0 siblings, 0 replies; 5+ results
From: Eric Wong @ 2017-03-26 3:37 UTC (permalink / raw)
To: Dylan Thacker-Smith; +Cc: unicorn-public
Dylan Thacker-Smith <dylan.smith@shopify.com> wrote:
> On Sat, Mar 25, 2017 at 10:41 PM, Eric Wong <e@80x24.org> wrote:
> > Is the last line to set "@@tcpi_inspect_ok = true" still
> > necessary? It was necessary in Jeremy's version, but I think
> > you can safely omit it, now.
>
> No it isn't needed, so you can remove it.
>
> I had just started my patch before seeing his, so reverted his as part
> of rebasing and this line was present before his patch.
Thanks, pushed as 477a2207869ff6b11a1cdee428b55604f2caa69e
to "master" of git://bogomips.org/unicorn
^ permalink raw reply [relevance 7%]
* [ANN] unicorn 5.3.0 - Rack HTTP server for fast clients and Unix
@ 2017-04-01 8:08 3% Eric Wong
0 siblings, 0 replies; 5+ results
From: Eric Wong @ 2017-04-01 8:08 UTC (permalink / raw)
To: ruby-talk, unicorn-public
Cc: Jeremy Evans, Simon Eskildsen, Dylan Thacker-Smith
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.
^ permalink raw reply [relevance 3%]
Results 1-5 of 5 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2017-03-26 1:49 6% [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO Dylan Thacker-Smith
2017-03-26 2:41 7% ` Eric Wong
2017-03-26 2:52 7% ` Dylan Thacker-Smith
2017-03-26 3:37 7% ` Eric Wong
2017-04-01 8:08 3% [ANN] unicorn 5.3.0 - Rack HTTP server for fast clients and Unix Eric Wong
Code repositories for project(s) associated with this public inbox
https://yhbt.net/unicorn.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).