Revert "Consolidate option handling in Server, Server small refactors, doc changes (#2373)"

This reverts commit bbbdfb8f4c.

lib/puma/runner.rb

@@ -54,8 +54,9 @@ module Puma
 
       app = Puma::App::Status.new @launcher, token
 
-      control = Puma::Server.new app, @launcher.events,
-        { min_threads: 0, max_threads: 1 }
+      control = Puma::Server.new app, @launcher.events
+      control.min_threads = 0
+      control.max_threads = 1
 
       control.binder.parse [str], self, 'Starting control server'
 
@@ -68,7 +69,6 @@ module Puma
       @control.binder.close_listeners if @control
     end
 
-    # @!attribute [r] ruby_engine
     def ruby_engine
       if !defined?(RUBY_ENGINE) || RUBY_ENGINE == "ruby"
         "ruby #{RUBY_VERSION}-p#{RUBY_PATCHLEVEL}"
@@ -137,14 +137,27 @@ module Puma
       @launcher.binder.parse @options[:binds], self
     end
 
-    # @!attribute [r] app
     def app
       @app ||= @launcher.config.app
     end
 
     def start_server
+      min_t = @options[:min_threads]
+      max_t = @options[:max_threads]
+
       server = Puma::Server.new app, @launcher.events, @options
+      server.min_threads = min_t
+      server.max_threads = max_t
       server.inherit_binder @launcher.binder
+
+      if @options[:early_hints]
+        server.early_hints = true
+      end
+
+      unless development? || test?
+        server.leak_stack_on_error = false
+      end
+
       server
     end
   end

lib/puma/server.rb

@@ -34,21 +34,15 @@ module Puma
 
     attr_reader :thread
     attr_reader :events
-    attr_reader :min_threads, :max_threads  # for #stats
-    attr_reader :requests_count             # @version 5.0.0
-
-    # the following may be deprecated in the future
-    attr_reader :auto_trim_time
-    attr_reader :first_data_timeout
-    attr_reader :persistent_timeout
-    attr_reader :reaping_time
-
+    attr_reader :requests_count # @version 5.0.0
     attr_accessor :app
-    attr_accessor :binder
 
-    def_delegators :@binder, :add_tcp_listener, :add_ssl_listener, :add_unix_listener, :connected_ports
-
-    ThreadLocalKey = :puma_server
+    attr_accessor :min_threads
+    attr_accessor :max_threads
+    attr_accessor :persistent_timeout
+    attr_accessor :auto_trim_time
+    attr_accessor :reaping_time
+    attr_accessor :first_data_timeout
 
     # Create a server for the rack app +app+.
     #
@@ -58,10 +52,6 @@ module Puma
     # Server#run returns a thread that you can join on to wait for the server
     # to do its work.
     #
-    # @note Several instance variables exist so they are available for testing,
-    #   and have default values set via +fetch+.  Normally the values are set via
-    #   `::Puma::Configuration.puma_default_options`.
-    #
     def initialize(app, events=Events.stdio, options={})
       @app = app
       @events = events
@@ -69,25 +59,25 @@ module Puma
       @check, @notify = nil
       @status = :stop
 
+      @min_threads = 0
+      @max_threads = 16
       @auto_trim_time = 30
       @reaping_time = 1
 
       @thread = nil
       @thread_pool = nil
+      @early_hints = nil
 
-      @options = options
-
-      @early_hints        = options.fetch :early_hints, nil
-      @first_data_timeout = options.fetch :first_data_timeout, FIRST_DATA_TIMEOUT
-      @min_threads        = options.fetch :min_threads, 0
-      @max_threads        = options.fetch :max_threads , (Puma.mri? ? 5 : 16)
-      @persistent_timeout = options.fetch :persistent_timeout, PERSISTENT_TIMEOUT
-      @queue_requests     = options.fetch :queue_requests, true
-
-      @leak_stack_on_error = !!(@options[:environment] =~ /\A(development|test)\z/)
+      @persistent_timeout = options.fetch(:persistent_timeout, PERSISTENT_TIMEOUT)
+      @first_data_timeout = options.fetch(:first_data_timeout, FIRST_DATA_TIMEOUT)
 
       @binder = Binder.new(events)
 
+      @leak_stack_on_error = true
+
+      @options = options
+      @queue_requests = options[:queue_requests].nil? ? true : options[:queue_requests]
+
       ENV['RACK_ENV'] ||= "development"
 
       @mode = :http
@@ -99,16 +89,15 @@ module Puma
       @shutdown_mutex = Mutex.new
     end
 
+    attr_accessor :binder, :leak_stack_on_error, :early_hints
+
+    def_delegators :@binder, :add_tcp_listener, :add_ssl_listener, :add_unix_listener, :connected_ports
+
     def inherit_binder(bind)
       @binder = bind
     end
 
     class << self
-      # @!attribute [r] current
-      def current
-        Thread.current[ThreadLocalKey]
-      end
-
       # :nodoc:
       # @version 5.0.0
       def tcp_cork_supported?
@@ -183,12 +172,10 @@ module Puma
       end
     end
 
-    # @!attribute [r] backlog
     def backlog
       @thread_pool and @thread_pool.backlog
     end
 
-    # @!attribute [r] running
     def running
       @thread_pool and @thread_pool.spawned
     end
@@ -201,7 +188,6 @@ module Puma
     # there are 5 threads sitting idle ready to take
     # a request. If one request comes in, then the
     # value would be 4 until it finishes processing.
-    # @!attribute [r] pool_capacity
     def pool_capacity
       @thread_pool and @thread_pool.pool_capacity
     end
@@ -1012,6 +998,12 @@ module Puma
     end
     private :fast_write
 
+    ThreadLocalKey = :puma_server
+
+    def self.current
+      Thread.current[ThreadLocalKey]
+    end
+
     def shutting_down?
       @status == :stop || @status == :restart
     end
@@ -1027,7 +1019,6 @@ module Puma
 
     # Returns a hash of stats about the running server for reporting purposes.
     # @version 5.0.0
-    # @!attribute [r] stats
     def stats
       STAT_METHODS.map {|name| [name, send(name) || 0]}.to_h
     end

test/test_busy_worker.rb

@@ -54,8 +54,8 @@ class TestBusyWorker < Minitest::Test
     end
 
     @server = Puma::Server.new request_handler, Puma::Events.strings, **options
-    @server.instance_variable_set :@min_threads, options[:min_threads] || 0
-    @server.instance_variable_set :@max_threads, options[:max_threads] || 10
+    @server.min_threads = options[:min_threads] || 0
+    @server.max_threads = options[:max_threads] || 10
     @port = (@server.add_tcp_listener '127.0.0.1', 0).addr[1]
     @server.run
   end

test/test_integration_pumactl.rb

@@ -108,7 +108,6 @@ class TestIntegrationPumactl < TestIntegration
     cli_pumactl "stop", unix: true
 
     _, status = Process.wait2(@pid)
-    assert_equal 0, status
     @server = nil
   end
 

test/test_out_of_band_server.rb

@@ -60,8 +60,8 @@ class TestOutOfBandServer < Minitest::Test
     end
 
     @server = Puma::Server.new app, Puma::Events.strings, out_of_band: [oob], **options
-    @server.instance_variable_set :@min_threads, options[:min_threads] || 1
-    @server.instance_variable_set :@max_threads, options[:max_threads] || 1
+    @server.min_threads = options[:min_threads] || 1
+    @server.max_threads = options[:max_threads] || 1
     @port = (@server.add_tcp_listener '127.0.0.1', 0).addr[1]
     @server.run
   end

test/test_persistent.rb

@@ -25,7 +25,7 @@ class TestPersistent < Minitest::Test
 
     @server = Puma::Server.new @simple
     @port = (@server.add_tcp_listener HOST, 0).addr[1]
-    @server.instance_variable_set :@max_threads, 1
+    @server.max_threads = 1
     @server.run
 
     @client = TCPSocket.new HOST, @port
@@ -152,7 +152,7 @@ class TestPersistent < Minitest::Test
   end
 
   def test_persistent_timeout
-    @server.instance_variable_set :@persistent_timeout, 1
+    @server.persistent_timeout = 1
     @client << @valid_request
     sz = @body[0].size.to_s
 
@@ -189,7 +189,7 @@ class TestPersistent < Minitest::Test
 
 
   def test_two_requests_in_one_chunk
-    @server.instance_variable_set :@persistent_timeout, 3
+    @server.persistent_timeout = 3
 
     req = @valid_request.to_s
     req += "GET /second HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\n\r\n"
@@ -206,7 +206,7 @@ class TestPersistent < Minitest::Test
   end
 
   def test_second_request_not_in_first_req_body
-    @server.instance_variable_set :@persistent_timeout, 3
+    @server.persistent_timeout = 3
 
     req = @valid_request.to_s
     req += "GET /second HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\n\r\n"

test/test_puma_server.rb

@@ -24,7 +24,7 @@ class TestPumaServer < Minitest::Test
   def server_run(app: @app, early_hints: false)
     @server.app = app
     @port = (@server.add_tcp_listener @host, 0).addr[1]
-    @server.instance_variable_set(:@early_hints, true) if early_hints
+    @server.early_hints = true if early_hints
     @server.run
   end
 
@@ -194,7 +194,7 @@ Content-Length: 12
 EOF
 ).split("\n").join("\r\n") + "\r\n\r\n"
 
-    assert_equal true, @server.instance_variable_get(:@early_hints)
+    assert_equal true, @server.early_hints
     assert_equal expected_data, data
   end
 
@@ -234,7 +234,7 @@ Content-Length: 12
 EOF
 ).split("\n").join("\r\n") + "\r\n\r\n"
 
-    assert_nil @server.instance_variable_get :@early_hints
+    assert_nil @server.early_hints
     assert_equal expected_data, data
   end
 
@@ -247,7 +247,7 @@ EOF
   end
 
   def test_doesnt_print_backtrace_in_production
-    @server.instance_variable_set :@leak_stack_on_error, false
+    @server.leak_stack_on_error = false
     server_run app: ->(env) { raise "don't leak me bro" }
 
     data = send_http_and_read "GET / HTTP/1.0\r\n\r\n"
@@ -284,8 +284,7 @@ EOF
   end
 
   def test_force_shutdown_error_default
-    @server = Puma::Server.new @app, @events, {force_shutdown_after: 2}
-    @server.instance_variable_set :@leak_stack_on_error, true
+    @server = Puma::Server.new @app, @events, {:force_shutdown_after => 2}
 
     server_run app: ->(env) do
       @server.stop
@@ -382,7 +381,7 @@ EOF
   end
 
   def test_timeout_in_data_phase
-    @server.instance_variable_set :@first_data_timeout, 2
+    @server.first_data_timeout = 2
     server_run
 
     sock = send_http "POST / HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\nContent-Length: 5\r\n\r\n"

test/test_pumactl.rb

@@ -182,7 +182,7 @@ class TestPumaControlCli < TestConfigFileBase
     out, _ = capture_subprocess_io do
       begin
         cmd.run
-      rescue SystemExit
+      rescue SystemExit => e
       end
     end
     assert_match expected_out, out