[Fix #3044] Set conditional config defaults after CLI options are parsed and config files are loaded

README.md

@@ -122,7 +122,7 @@ For an in-depth discussion of the tradeoffs of thread and process count settings
 
 In clustered mode, Puma can "preload" your application. This loads all the application code *prior* to forking. Preloading reduces total memory usage of your application via an operating system feature called [copy-on-write](https://en.wikipedia.org/wiki/Copy-on-write).
 
-If the `WEB_CONCURRENCY` environment variable is set to a value > 1 (and `--prune-bundler` has not been specified), preloading will be enabled by default. Otherwise, you can use the `--preload` flag from the command line:
+If the number of workers is greater than 1 (and `--prune-bundler` has not been specified), preloading will be enabled by default. Otherwise, you can use the `--preload` flag from the command line:
 
 ```
 $ puma -w 3 --preload

lib/puma/cli.rb

@@ -39,10 +39,8 @@ def initialize(argv, log_writer = LogWriter.stdio, events = Events.new)
       @control_url = nil
       @control_options = {}
 
-      setup_options
-
       begin
-        @parser.parse! @argv
+        setup_options
 
         if file = @argv.shift
           @conf.configure do |user_config, file_config|
@@ -240,7 +238,7 @@ def setup_options
             $stdout.puts o
             exit 0
           end
-        end
+        end.parse! @argv
       end
     end
   end

lib/puma/configuration.rb

@@ -181,13 +181,11 @@ def initialize(user_options={}, default_options = {}, &block)
       @file_dsl    = DSL.new(@options.file_options, self)
       @default_dsl = DSL.new(@options.default_options, self)
 
-      if !@options[:prune_bundler]
-        default_options[:preload_app] = (@options[:workers] > 1) && Puma.forkable?
-      end
-
       if block
         configure(&block)
       end
+
+      set_conditional_default_options
     end
 
     attr_reader :options, :plugins
@@ -237,6 +235,8 @@ def puma_options_from_env
     def load
       config_files.each { |config_file| @file_dsl._load_from(config_file) }
 
+      set_conditional_default_options
+
       @options
     end
 
@@ -378,6 +378,12 @@ def load_rackup
       rack_app
     end
 
+    def set_conditional_default_options
+      @options.default_options[:preload_app] = !@options[:prune_bundler] &&
+        (@options[:workers] > 1) &&
+        Puma.forkable?
+    end
+
     def self.random_token
       require 'securerandom' unless defined?(SecureRandom)
 

test/config/worker_shutdown_timeout_2.rb → test/config/worker_respawn.rb

@@ -1 +1,2 @@
 worker_shutdown_timeout 2
+preload_app! false

test/config/workers_0.rb

@@ -0,0 +1 @@
+workers 0

test/config/workers_2.rb

@@ -0,0 +1 @@
+workers 2

test/test_cli.rb

@@ -352,7 +352,6 @@ def test_environment_app_env
     ENV['APP_ENV'] = 'test'
 
     cli = Puma::CLI.new []
-    cli.send(:setup_options)
 
     assert_equal 'test', cli.instance_variable_get(:@conf).environment
   ensure
@@ -364,7 +363,6 @@ def test_environment_rack_env
     ENV['RACK_ENV'] = @environment
 
     cli = Puma::CLI.new []
-    cli.send(:setup_options)
 
     assert_equal @environment, cli.instance_variable_get(:@conf).environment
   end
@@ -374,7 +372,6 @@ def test_environment_rails_env
     ENV['RAILS_ENV'] = @environment
 
     cli = Puma::CLI.new []
-    cli.send(:setup_options)
 
     assert_equal @environment, cli.instance_variable_get(:@conf).environment
   ensure
@@ -383,7 +380,6 @@ def test_environment_rails_env
 
   def test_silent
     cli = Puma::CLI.new ['--silent']
-    cli.send(:setup_options)
 
     log_writer = cli.instance_variable_get(:@log_writer)
 
@@ -396,9 +392,26 @@ def test_plugins
     assert_empty Puma::Plugins.instance_variable_get(:@plugins)
 
     cli = Puma::CLI.new ['--plugin', 'tmp_restart', '--plugin', 'systemd']
-    cli.send(:setup_options)
 
     assert Puma::Plugins.find("tmp_restart")
     assert Puma::Plugins.find("systemd")
   end
+
+  def test_config_does_not_preload_app_with_workers
+    skip_unless :fork
+
+    cli = Puma::CLI.new ['-w 0']
+    config = Puma.cli_config
+
+    assert_equal false, config.options[:preload_app]
+  end
+
+  def test_config_preloads_app_with_workers
+    skip_unless :fork
+
+    cli = Puma::CLI.new ['-w 2']
+    config = Puma.cli_config
+
+    assert_equal true, config.options[:preload_app]
+  end
 end

test/test_config.rb

@@ -576,6 +576,34 @@ def test_http_content_length_limit
     assert_equal 10000, conf.final_options[:http_content_length_limit]
   end
 
+  def test_config_does_not_preload_app_if_not_using_workers
+    conf = Puma::Configuration.new({ workers: 0 })
+
+    assert_equal false, conf.options.default_options[:preload_app]
+  end
+
+  def test_config_preloads_app_if_using_workers
+    conf = Puma::Configuration.new({ workers: 2 })
+    preload = Puma.forkable?
+
+    assert_equal preload, conf.options.default_options[:preload_app]
+  end
+
+  def test_config_file_does_not_preload_app_if_not_using_workers
+    conf = Puma::Configuration.new { |c| c.load 'test/config/workers_0.rb' }
+    conf.load
+
+    assert_equal false, conf.options.default_options[:preload_app]
+  end
+
+  def test_config_file_preloads_app_if_using_workers
+    conf = Puma::Configuration.new { |c| c.load 'test/config/workers_2.rb' }
+    conf.load
+    preload = Puma.forkable?
+
+    assert_equal preload, conf.options.default_options[:preload_app]
+  end
+
   private
 
   def assert_run_hooks(hook_name, options = {})

test/test_integration_cluster.rb

@@ -104,17 +104,17 @@ def test_term_closes_listeners_unix
 
   def test_usr1_all_respond_tcp
     skip_unless_signal_exist? :USR1
-    usr1_all_respond unix: false
+    usr1_all_respond unix: false, config: 'preload_app! false'
   end
 
   def test_usr1_fork_worker
     skip_unless_signal_exist? :USR1
-    usr1_all_respond config: '--fork-worker'
+    usr1_all_respond config: 'fork_worker'
   end
 
   def test_usr1_all_respond_unix
     skip_unless_signal_exist? :USR1
-    usr1_all_respond unix: true
+    usr1_all_respond unix: true, config: 'preload_app! false'
   end
 
   def test_term_exit_code
@@ -343,6 +343,7 @@ def test_fork_worker_phased_restart_with_high_worker_count
       # to simulate worker 0 timeout, total boot time for all workers
       # needs to exceed single worker timeout
       workers #{worker_count}
+      preload_app! false
     RUBY
 
     # workers is the default
@@ -661,7 +662,7 @@ def term_closes_listeners(unix: false)
   # Send requests 1 per second.  Send 1, then :USR1 server, then send another 24.
   # All should be responded to, and at least three workers should be used
   def usr1_all_respond(unix: false, config: '')
-    cli_server "-w #{workers} -t 0:5 -q test/rackup/sleep_pid.ru #{config}", unix: unix
+    cli_server "-w #{workers} -t 0:5 -q test/rackup/sleep_pid.ru", unix: unix, config: config
     threads = []
     replies = []
     mutex = Mutex.new
@@ -709,7 +710,7 @@ def usr1_all_respond(unix: false, config: '')
     end
   end
 
-  def worker_respawn(phase = 1, size = workers, config = 'test/config/worker_shutdown_timeout_2.rb')
+  def worker_respawn(phase = 1, size = workers, config = 'test/config/worker_respawn.rb')
     threads = []
 
     cli_server "-w #{workers} -t 1:1 -C #{config} test/rackup/sleep_pid.ru"

test/test_integration_pumactl.rb

@@ -60,7 +60,12 @@ def ctl_unix(signal='stop')
 
   def test_phased_restart_cluster
     skip_unless :fork
-    cli_server "-q -w #{workers} test/rackup/sleep.ru #{set_pumactl_args unix: true} -S #{@state_path}", unix: true
+    cli_server "test/rackup/sleep.ru #{set_pumactl_args unix: true}", unix: true, config: <<~RUBY
+      quiet
+      workers #{workers}
+      preload_app! false
+      state_path "#{@state_path}"
+    RUBY
 
     start = Process.clock_gettime(Process::CLOCK_MONOTONIC)