* [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO
@ 2017-03-26 1:49 Dylan Thacker-Smith
2017-03-26 2:41 ` Eric Wong
0 siblings, 1 reply; 4+ messages in thread
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 [flat|nested] 4+ messages in thread
* Re: [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO
2017-03-26 1:49 [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO Dylan Thacker-Smith
@ 2017-03-26 2:41 ` Eric Wong
2017-03-26 2:52 ` Dylan Thacker-Smith
0 siblings, 1 reply; 4+ messages in thread
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 [flat|nested] 4+ messages in thread
* Re: [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO
2017-03-26 2:41 ` Eric Wong
@ 2017-03-26 2:52 ` Dylan Thacker-Smith
2017-03-26 3:37 ` Eric Wong
0 siblings, 1 reply; 4+ messages in thread
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 [flat|nested] 4+ messages in thread
* Re: [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO
2017-03-26 2:52 ` Dylan Thacker-Smith
@ 2017-03-26 3:37 ` Eric Wong
0 siblings, 0 replies; 4+ messages in thread
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 [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-26 3:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-26 1:49 [PATCH] Check for Socket::TCP_INFO constant before trying to get TCP_INFO Dylan Thacker-Smith
2017-03-26 2:41 ` Eric Wong
2017-03-26 2:52 ` Dylan Thacker-Smith
2017-03-26 3:37 ` 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).