From b652fa51c1342496bdcdecca8e567f1fb46c41c9 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 5 Sep 2023 06:43:20 +0000 Subject: kill off remaining kgio uses kgio is an extra download and shared object which costs users bandwidth, disk space, startup time and memory. Ruby 2.3+ provides `Socket#accept_nonblock(exception: false)' support in addition to `exception: false' support in IO#*_nonblock methods from Ruby 2.1. We no longer distinguish between TCPServer and UNIXServer as separate classes internally; instead favoring the `Socket' class of Ruby for both. This allows us to use `Socket#accept_nonblock' and get a populated `Addrinfo' object off accept4(2)/accept(2) without resorting to a getpeername(2) syscall (kgio avoided getpeername(2) in the same way). The downside is there's more Ruby-level argument passing and stack usage on our end with HttpRequest#read_headers (formerly HttpRequest#read). I chose this tradeoff since advancements in Ruby itself can theoretically mitigate the cost of argument passing, while syscalls are a high fixed cost given modern CPU vulnerability mitigations. Note: no benchmarks have been run since I don't have a suitable system. --- lib/unicorn.rb | 3 +-- lib/unicorn/http_request.rb | 14 ++++++------- lib/unicorn/http_server.rb | 30 ++++++++------------------ lib/unicorn/oob_gc.rb | 4 ++-- lib/unicorn/socket_helper.rb | 50 +++++--------------------------------------- lib/unicorn/worker.rb | 6 +++--- 6 files changed, 27 insertions(+), 80 deletions(-) (limited to 'lib') diff --git a/lib/unicorn.rb b/lib/unicorn.rb index b817b77..564cb30 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -1,7 +1,6 @@ # -*- encoding: binary -*- require 'etc' require 'stringio' -require 'kgio' require 'raindrops' require 'io/wait' @@ -112,7 +111,7 @@ module Unicorn F_SETPIPE_SZ = 1031 if RUBY_PLATFORM =~ /linux/ def self.pipe # :nodoc: - Kgio::Pipe.new.each do |io| + IO.pipe.each do |io| # shrink pipes to minimize impact on /proc/sys/fs/pipe-user-pages-soft # limits. if defined?(F_SETPIPE_SZ) diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb index 8bca60a..ab3bd6e 100644 --- a/lib/unicorn/http_request.rb +++ b/lib/unicorn/http_request.rb @@ -61,7 +61,7 @@ class Unicorn::HttpParser # 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_headers(socket, ai) e = env # From https://www.ietf.org/rfc/rfc3875: @@ -71,7 +71,7 @@ class Unicorn::HttpParser # identify the client for the immediate request to the server; # that client may be a proxy, gateway, or other intermediary # acting on behalf of the actual source client." - e['REMOTE_ADDR'] = socket.kgio_addr + e['REMOTE_ADDR'] = ai.unix? ? '127.0.0.1' : ai.ip_address # short circuit the common case with small GET requests first socket.readpartial(16384, buf) @@ -81,7 +81,7 @@ class Unicorn::HttpParser false until add_parse(socket.readpartial(16384)) end - check_client_connection(socket) if @@check_client_connection + check_client_connection(socket, ai) if @@check_client_connection e['rack.input'] = 0 == content_length ? NULL_IO : @@input_class.new(socket, self) @@ -107,8 +107,8 @@ class Unicorn::HttpParser if Raindrops.const_defined?(:TCP_Info) TCPI = Raindrops::TCP_Info.allocate - def check_client_connection(socket) # :nodoc: - if Unicorn::TCPClient === socket + def check_client_connection(socket, ai) # :nodoc: + if ai.ip? # Raindrops::TCP_Info#get!, #state (reads struct tcp_info#tcpi_state) raise Errno::EPIPE, "client closed connection".freeze, EMPTY_ARRAY if closed_state?(TCPI.get!(socket).state) @@ -152,8 +152,8 @@ class Unicorn::HttpParser # Ruby 2.2+ can show struct tcp_info as a string Socket::Option#inspect. # 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 + def check_client_connection(socket, ai) # :nodoc: + if @@tcpi_inspect_ok && ai.ip? opt = socket.getsockopt(Socket::IPPROTO_TCP, Socket::TCP_INFO).inspect if opt =~ /\bstate=(\S+)/ raise Errno::EPIPE, "client closed connection".freeze, diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb index 2f1eb1b..ed5bbf1 100644 --- a/lib/unicorn/http_server.rb +++ b/lib/unicorn/http_server.rb @@ -111,9 +111,7 @@ class Unicorn::HttpServer @worker_data = if worker_data = ENV['UNICORN_WORKER'] worker_data = worker_data.split(',').map!(&:to_i) - worker_data[1] = worker_data.slice!(1..2).map do |i| - Kgio::Pipe.for_fd(i) - end + worker_data[1] = worker_data.slice!(1..2).map { |i| IO.for_fd(i) } worker_data end end @@ -243,10 +241,6 @@ class Unicorn::HttpServer tries = opt[:tries] || 5 begin io = bind_listen(address, opt) - unless Kgio::TCPServer === io || Kgio::UNIXServer === io - io.autoclose = false - io = server_cast(io) - end logger.info "listening on addr=#{sock_name(io)} fd=#{io.fileno}" LISTENERS << io io @@ -594,9 +588,9 @@ class Unicorn::HttpServer # 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) + def process_client(client, ai) @request = Unicorn::HttpRequest.new - env = @request.read(client) + env = @request.read_headers(client, ai) if early_hints env["rack.early_hints"] = lambda do |headers| @@ -708,10 +702,9 @@ class Unicorn::HttpServer reopen = reopen_worker_logs(worker.nr) if reopen worker.tick = time_now.to_i while sock = ready.shift - # Unicorn::Worker#kgio_tryaccept is not like accept(2) at all, - # but that will return false - if client = sock.kgio_tryaccept - process_client(client) + client_ai = sock.accept_nonblock(exception: false) + if client_ai != :wait_readable + process_client(*client_ai) worker.tick = time_now.to_i end break if reopen @@ -809,7 +802,6 @@ class Unicorn::HttpServer def inherit_listeners! # inherit sockets from parents, they need to be plain Socket objects - # before they become Kgio::UNIXServer or Kgio::TCPServer inherited = ENV['UNICORN_FD'].to_s.split(',') immortal = [] @@ -825,8 +817,6 @@ class Unicorn::HttpServer inherited.map! do |fd| io = Socket.for_fd(fd.to_i) @immortal << io if immortal.include?(fd) - io.autoclose = false - io = server_cast(io) set_server_sockopt(io, listener_opts[sock_name(io)]) logger.info "inherited addr=#{sock_name(io)} fd=#{io.fileno}" io @@ -835,11 +825,9 @@ class Unicorn::HttpServer config_listeners = config[:listeners].dup LISTENERS.replace(inherited) - # we start out with generic Socket objects that get cast to either - # Kgio::TCPServer or Kgio::UNIXServer objects; but since the Socket - # objects share the same OS-level file descriptor as the higher-level - # *Server objects; we need to prevent Socket objects from being - # garbage-collected + # we only use generic Socket objects for aggregate Socket#accept_nonblock + # return value [ Socket, Addrinfo ]. This allows us to avoid having to + # make getpeername(2) syscalls later on to fill in env['REMOTE_ADDR'] config_listeners -= listener_names if config_listeners.empty? && LISTENERS.empty? config_listeners << Unicorn::Const::DEFAULT_LISTEN diff --git a/lib/unicorn/oob_gc.rb b/lib/unicorn/oob_gc.rb index 91a8e51..db9f2cb 100644 --- a/lib/unicorn/oob_gc.rb +++ b/lib/unicorn/oob_gc.rb @@ -65,8 +65,8 @@ module Unicorn::OobGC end #:stopdoc: - def process_client(client) - super(client) # Unicorn::HttpServer#process_client + def process_client(*args) + super(*args) # Unicorn::HttpServer#process_client env = instance_variable_get(:@request).env if OOBGC_PATH =~ env['PATH_INFO'] && ((@@nr -= 1) <= 0) @@nr = OOBGC_INTERVAL diff --git a/lib/unicorn/socket_helper.rb b/lib/unicorn/socket_helper.rb index c2ba75e..06ec2b2 100644 --- a/lib/unicorn/socket_helper.rb +++ b/lib/unicorn/socket_helper.rb @@ -3,32 +3,6 @@ require 'socket' module Unicorn - - # Instead of using a generic Kgio::Socket for everything, - # tag TCP sockets so we can use TCP_INFO under Linux without - # incurring extra syscalls for Unix domain sockets. - # TODO: remove these when we remove kgio - TCPClient = Class.new(Kgio::Socket) # :nodoc: - class TCPSrv < Kgio::TCPServer # :nodoc: - def kgio_tryaccept # :nodoc: - super(TCPClient) - end - end - - if IO.instance_method(:write).arity == 1 # Ruby <= 2.4 - require 'unicorn/write_splat' - UNIXClient = Class.new(Kgio::Socket) # :nodoc: - class UNIXSrv < Kgio::UNIXServer # :nodoc: - include Unicorn::WriteSplat - def kgio_tryaccept # :nodoc: - super(UNIXClient) - end - end - TCPClient.__send__(:include, Unicorn::WriteSplat) - else # Ruby 2.5+ - UNIXSrv = Kgio::UNIXServer - end - module SocketHelper # internal interface @@ -105,7 +79,7 @@ module Unicorn def set_server_sockopt(sock, opt) opt = DEFAULTS.merge(opt || {}) - TCPSocket === sock and set_tcp_sockopt(sock, opt) + set_tcp_sockopt(sock, opt) if sock.local_address.ip? rcvbuf, sndbuf = opt.values_at(:rcvbuf, :sndbuf) if rcvbuf || sndbuf @@ -149,7 +123,9 @@ module Unicorn end old_umask = File.umask(opt[:umask] || 0) begin - UNIXSrv.new(address) + s = Socket.new(:UNIX, :STREAM) + s.bind(Socket.sockaddr_un(address)) + s ensure File.umask(old_umask) end @@ -177,8 +153,7 @@ module Unicorn sock.setsockopt(:SOL_SOCKET, :SO_REUSEPORT, 1) end sock.bind(Socket.pack_sockaddr_in(port, addr)) - sock.autoclose = false - TCPSrv.for_fd(sock.fileno) + sock end # returns rfc2732-style (e.g. "[::1]:666") addresses for IPv6 @@ -194,10 +169,6 @@ module Unicorn def sock_name(sock) case sock when String then sock - when UNIXServer - Socket.unpack_sockaddr_un(sock.getsockname) - when TCPServer - tcp_name(sock) when Socket begin tcp_name(sock) @@ -210,16 +181,5 @@ module Unicorn end module_function :sock_name - - # casts a given Socket to be a TCPServer or UNIXServer - def server_cast(sock) - begin - Socket.unpack_sockaddr_in(sock.getsockname) - TCPSrv.for_fd(sock.fileno) - rescue ArgumentError - UNIXSrv.for_fd(sock.fileno) - end - end - end # module SocketHelper end # module Unicorn diff --git a/lib/unicorn/worker.rb b/lib/unicorn/worker.rb index ad5814c..4af31be 100644 --- a/lib/unicorn/worker.rb +++ b/lib/unicorn/worker.rb @@ -71,8 +71,8 @@ class Unicorn::Worker end # this only runs when the Rack app.call is not running - # act like a listener - def kgio_tryaccept # :nodoc: + # act like Socket#accept_nonblock(exception: false) + def accept_nonblock(*_unused) # :nodoc: case buf = @to_io.read_nonblock(4, exception: false) when String # unpack the buffer and trigger the signal handler @@ -82,7 +82,7 @@ class Unicorn::Worker when nil # EOF: master died, but we are at a safe place to exit fake_sig(:QUIT) when :wait_readable # keep waiting - return false + return :wait_readable end while true # loop, as multiple signals may be sent end -- cgit v1.2.3-24-ge0c7