From 4ac14482f1eda4bcf2d2baa3a379afe3f5b55a9c Mon Sep 17 00:00:00 2001 From: Dalibor Nasevic Date: Sat, 1 Jan 2022 23:11:22 +0100 Subject: [PATCH] Allow backlog parameter to be set with ssl_bind DSL (#2780) Hint about backlog upper limit due to net.core.somaxconn Guard against invalid backlog value --- docs/architecture.md | 8 ++++---- lib/puma/binder.rb | 7 ++++--- lib/puma/dsl.rb | 6 ++++-- test/test_binder.rb | 18 ++++++++++++++++++ test/test_config.rb | 15 +++++++++++++++ test/test_thread_pool.rb | 2 +- 6 files changed, 46 insertions(+), 10 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index e4fb5710..83f438b7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -31,10 +31,10 @@ _workers_, and we sometimes call the threads created by Puma's ![https://bit.ly/2zwzhEK](images/puma-connection-flow.png) * Upon startup, Puma listens on a TCP or UNIX socket. - * The backlog of this socket is configured (with a default of 1024). The - backlog determines the size of the queue for unaccepted connections. - Generally, you'll never hit the backlog cap in production. If the backlog is - full, the operating system refuses new connections. + * The backlog of this socket is configured with a default of 1024, but the + actual backlog value is capped by the `net.core.somaxconn` sysctl value. + The backlog determines the size of the queue for unaccepted connections. If + the backlog is full, the operating system is not accepting new connections. * This socket backlog is distinct from the `backlog` of work as reported by `Puma.stats` or the control server. The backlog that `Puma.stats` refers to represents the number of connections in the process' `todo` set waiting for diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index 3d688296..a9d2bda3 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -168,9 +168,9 @@ module Puma params = Util.parse_query uri.query opt = params.key?('low_latency') && params['low_latency'] != 'false' - bak = params.fetch('backlog', 1024).to_i + backlog = params.fetch('backlog', 1024).to_i - io = add_tcp_listener uri.host, uri.port, opt, bak + io = add_tcp_listener uri.host, uri.port, opt, backlog @ios[ios_len..-1].each do |i| addr = loc_addr_str i @@ -255,7 +255,8 @@ module Puma logger.log "* Activated #{str}" else ios_len = @ios.length - io = add_ssl_listener uri.host, uri.port, ctx + backlog = params.fetch('backlog', 1024).to_i + io = add_ssl_listener uri.host, uri.port, ctx, optimize_for_latency = true, backlog @ios[ios_len..-1].each do |i| addr = loc_addr_str i diff --git a/lib/puma/dsl.rb b/lib/puma/dsl.rb index a221587b..9a0573a7 100644 --- a/lib/puma/dsl.rb +++ b/lib/puma/dsl.rb @@ -48,6 +48,8 @@ module Puma ca_additions = "&ca=#{opts[:ca]}" if ['peer', 'force_peer'].include?(verify) + backlog_str = opts[:backlog] ? "&backlog=#{Integer(opts[:backlog])}" : '' + if defined?(JRUBY_VERSION) ssl_cipher_list = opts[:ssl_cipher_list] ? "&ssl_cipher_list=#{opts[:ssl_cipher_list]}" : nil @@ -55,7 +57,7 @@ module Puma keystore_additions = "keystore=#{opts[:keystore]}&keystore-pass=#{opts[:keystore_pass]}" "ssl://#{host}:#{port}?#{keystore_additions}#{ssl_cipher_list}" \ - "&verify_mode=#{verify}#{tls_str}#{ca_additions}" + "&verify_mode=#{verify}#{tls_str}#{ca_additions}#{backlog_str}" else ssl_cipher_filter = opts[:ssl_cipher_filter] ? "&ssl_cipher_filter=#{opts[:ssl_cipher_filter]}" : nil @@ -64,7 +66,7 @@ module Puma "&verification_flags=#{Array(ary).join ','}" : nil "ssl://#{host}:#{port}?cert=#{opts[:cert]}&key=#{opts[:key]}" \ - "#{ssl_cipher_filter}&verify_mode=#{verify}#{tls_str}#{ca_additions}#{v_flags}" + "#{ssl_cipher_filter}&verify_mode=#{verify}#{tls_str}#{ca_additions}#{v_flags}#{backlog_str}" end end diff --git a/test/test_binder.rb b/test/test_binder.rb index 4dddbcce..c2fa42b8 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -275,6 +275,24 @@ class TestBinder < TestBinderBase assert_equal @events.stderr, env_hash["rack.errors"] end + def test_ssl_binder_sets_backlog + skip_unless :ssl + + host = '127.0.0.1' + port = UniquePort.call + tcp_server = TCPServer.new(host, port) + tcp_server.define_singleton_method(:listen) do |backlog| + Thread.current[:backlog] = backlog + super(backlog) + end + + TCPServer.stub(:new, tcp_server) do + @binder.parse ["ssl://#{host}:#{port}?#{ssl_query}&backlog=2048"], @events + end + + assert_equal 2048, Thread.current[:backlog] + end + def test_close_calls_close_on_ios @mocked_ios = [Minitest::Mock.new, Minitest::Mock.new] @mocked_ios.each { |m| m.expect(:close, true) } diff --git a/test/test_config.rb b/test/test_config.rb index b8958d09..90ace92c 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -98,6 +98,21 @@ class TestConfigFile < TestConfigFileBase assert_equal [ssl_binding], conf.options[:binds] end + def test_ssl_bind_with_backlog + skip_unless :ssl + + conf = Puma::Configuration.new do |c| + c.ssl_bind "0.0.0.0", "9292", { + backlog: "2048", + } + end + + conf.load + + ssl_binding = conf.options[:binds].first + assert ssl_binding.include?('&backlog=2048') + end + def test_ssl_bind_jruby skip_unless :jruby skip_unless :ssl diff --git a/test/test_thread_pool.rb b/test/test_thread_pool.rb index 2639eede..8021e7be 100644 --- a/test/test_thread_pool.rb +++ b/test/test_thread_pool.rb @@ -5,7 +5,7 @@ require "puma/thread_pool" class TestThreadPool < Minitest::Test def teardown - @pool.shutdown(1) if @pool + @pool.shutdown(1) if defined?(@pool) end def new_pool(min, max, &block)