From 5f39ca857724ff59f9f45542287510198e21e4e7 Mon Sep 17 00:00:00 2001 From: Will Jordan Date: Fri, 7 Feb 2020 12:42:34 -0800 Subject: [PATCH 1/3] Simplify Runner#start_control URL parsing Reuse logic from Binder#parse. --- History.md | 1 + lib/puma/runner.rb | 22 +--------------------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/History.md b/History.md index 3d06bb008e..960c0c17ab 100644 --- a/History.md +++ b/History.md @@ -13,6 +13,7 @@ * Refactor * Remove unused loader argument from Plugin initializer (#2095) * Simplify `Configuration.random_token` and remove insecure fallback (#2102) + * Simplify `Runner#start_control` URL parsing (#2111) ## 4.3.1 and 3.12.2 / 2019-12-05 diff --git a/lib/puma/runner.rb b/lib/puma/runner.rb index 2c9812fd92..90ceb7c83d 100644 --- a/lib/puma/runner.rb +++ b/lib/puma/runner.rb @@ -52,8 +52,6 @@ def start_control require 'puma/app/status' - uri = URI.parse str - if token = @options[:control_auth_token] token = nil if token.empty? || token == 'none' end @@ -64,25 +62,7 @@ def start_control control.min_threads = 0 control.max_threads = 1 - case uri.scheme - when "ssl" - log "* Starting control server on #{str}" - params = Util.parse_query uri.query - ctx = MiniSSL::ContextBuilder.new(params, @events).context - - control.add_ssl_listener uri.host, uri.port, ctx - when "tcp" - log "* Starting control server on #{str}" - control.add_tcp_listener uri.host, uri.port - when "unix" - log "* Starting control server on #{str}" - path = "#{uri.host}#{uri.path}" - mask = @options[:control_url_umask] - - control.add_unix_listener path, mask - else - error "Invalid control URI: #{str}" - end + control.binder.parse [str], self control.run @control = control From f3c3bf540f1892572fec290f75bc48d1ba8fa381 Mon Sep 17 00:00:00 2001 From: Will Jordan Date: Fri, 7 Feb 2020 17:49:08 -0800 Subject: [PATCH 2/3] Parameterize log message --- lib/puma/binder.rb | 6 +++--- lib/puma/runner.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index 28fcb5e47a..aa0e1d7e78 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -86,7 +86,7 @@ def import_from_env end end - def parse(binds, logger) + def parse(binds, logger, log_msg = 'Listening') binds.each do |str| uri = URI.parse str case uri.scheme @@ -113,7 +113,7 @@ def parse(binds, logger) i.local_address.ip_unpack.join(':') end - logger.log "* Listening on tcp://#{addr}" + logger.log "* #{log_msg} on tcp://#{addr}" end end @@ -149,7 +149,7 @@ def parse(binds, logger) end io = add_unix_listener path, umask, mode, backlog - logger.log "* Listening on #{str}" + logger.log "* #{log_msg} on #{str}" end @listeners << [str, io] diff --git a/lib/puma/runner.rb b/lib/puma/runner.rb index 90ceb7c83d..078a26d760 100644 --- a/lib/puma/runner.rb +++ b/lib/puma/runner.rb @@ -62,7 +62,7 @@ def start_control control.min_threads = 0 control.max_threads = 1 - control.binder.parse [str], self + control.binder.parse [str], self, 'Starting control server' control.run @control = control From 493b3f5652c5b1f612a65c2c0d3cfd7dea3d7a2f Mon Sep 17 00:00:00 2001 From: Will Jordan Date: Tue, 11 Feb 2020 11:05:06 -0800 Subject: [PATCH 3/3] Ensure control server Unix socket is closed on shutdown Remove Binder#close_unix_paths --- History.md | 1 + lib/puma/binder.rb | 4 ---- lib/puma/launcher.rb | 3 ++- lib/puma/runner.rb | 4 ++++ test/test_binder.rb | 4 ++-- test/test_integration_pumactl.rb | 12 +++++++++++- 6 files changed, 20 insertions(+), 8 deletions(-) diff --git a/History.md b/History.md index 960c0c17ab..9b906634a2 100644 --- a/History.md +++ b/History.md @@ -9,6 +9,7 @@ * Bugfixes * Your bugfix goes here (#Github Number) * Windows update extconf.rb for use with ssp and varied Ruby/MSYS2 combinations (#2069) + * Ensure control server Unix socket is closed on shutdown (#2112) * Refactor * Remove unused loader argument from Plugin initializer (#2095) diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index aa0e1d7e78..d36aaaaf00 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -369,10 +369,6 @@ def close_listeners end end - def close_unix_paths - @unix_paths.each { |up| File.unlink(up) if File.exist? up } - end - def redirects_for_restart redirects = {:close_others => true} @listeners.each_with_index do |(l, io), i| diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 4b96588a57..f50ee7b0f0 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -184,7 +184,7 @@ def run when :exit # nothing end - @binder.close_unix_paths + close_binder_listeners unless @status == :restart end # Return which tcp port the launcher is using, if it's using TCP @@ -202,6 +202,7 @@ def restart_args end def close_binder_listeners + @runner.close_control_listeners @binder.close_listeners end diff --git a/lib/puma/runner.rb b/lib/puma/runner.rb index 078a26d760..4de9a6f213 100644 --- a/lib/puma/runner.rb +++ b/lib/puma/runner.rb @@ -68,6 +68,10 @@ def start_control @control = control end + def close_control_listeners + @control.binder.close_listeners if @control + end + def ruby_engine if !defined?(RUBY_ENGINE) || RUBY_ENGINE == "ruby" "ruby #{RUBY_VERSION}-p#{RUBY_PATCHLEVEL}" diff --git a/test/test_binder.rb b/test/test_binder.rb index 7bd930946c..c16330703f 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -90,7 +90,7 @@ def test_pre_existing_unix refute_includes @binder.instance_variable_get(:@unix_paths), unix_path - @binder.close_unix_paths + @binder.close_listeners assert File.exist?(unix_path) @@ -150,7 +150,7 @@ def assert_parsing_logs_uri(order = [:unix, :tcp]) assert stdout.include?(prepared_paths[order[0]]), "\n#{stdout}\n" assert stdout.include?(prepared_paths[order[1]]), "\n#{stdout}\n" ensure - @binder.close_unix_paths if order.include?(:unix) && UNIX_SKT_EXIST + @binder.close_listeners if order.include?(:unix) && UNIX_SKT_EXIST end end diff --git a/test/test_integration_pumactl.rb b/test/test_integration_pumactl.rb index b7ae5f715f..e294bc3ab5 100644 --- a/test/test_integration_pumactl.rb +++ b/test/test_integration_pumactl.rb @@ -14,6 +14,8 @@ def setup def teardown super + refute File.exist?(@control_path), "Control path must be removed after stop" + ensure [@state_path, @control_path].each { |p| File.unlink(p) rescue nil } end @@ -30,10 +32,18 @@ def test_stop_tcp end def test_stop_unix + ctl_unix + end + + def test_halt_unix + ctl_unix 'halt' + end + + def ctl_unix(signal='stop') skip UNIX_SKT_MSG unless UNIX_SKT_EXIST cli_server "-q test/rackup/sleep.ru --control-url unix://#{@control_path} --control-token #{TOKEN} -S #{@state_path}", unix: true - cli_pumactl "stop", unix: true + cli_pumactl signal, unix: true _, status = Process.wait2(@pid) assert_equal 0, status