From 1a484d8d1c4007572cea394279109246de4395da Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Tue, 1 Oct 2019 08:50:38 -0500 Subject: [PATCH 1/4] Fix Binder @unix_paths handling 1. Move removal to launcher from single and cluster. 2. @unix_paths only contains files that Puma creates (pre-existing files are not) --- lib/puma/binder.rb | 7 ++++--- lib/puma/cluster.rb | 1 - lib/puma/launcher.rb | 5 +---- lib/puma/single.rb | 1 - 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index a5e48c8c28..e1156d6dec 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -361,7 +361,7 @@ def inherit_ssl_listener(fd, ctx) # Tell the server to listen on +path+ as a UNIX domain socket. # def add_unix_listener(path, umask=nil, mode=nil, backlog=1024) - @unix_paths << path + @unix_paths << path unless File.exist? path # Let anyone connect by default umask ||= 0 @@ -399,7 +399,7 @@ def add_unix_listener(path, umask=nil, mode=nil, backlog=1024) end def inherit_unix_listener(path, fd) - @unix_paths << path + @unix_paths << path unless File.exist? path if fd.kind_of? TCPServer s = fd @@ -420,7 +420,8 @@ def close_listeners io.close uri = URI.parse(l) next unless uri.scheme == 'unix' - File.unlink("#{uri.host}#{uri.path}") + unix_path = "#{uri.host}#{uri.path}" + File.unlink unix_path if @unix_paths.include? unix_path end end diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index 5b84f66853..fb9b63fa0b 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -529,7 +529,6 @@ def run @suicide_pipe.close read.close @wakeup.close - @launcher.close_binder_unix_paths end end diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 4f896c8591..2a0cd9be39 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -184,6 +184,7 @@ def run when :exit # nothing end + @binder.close_unix_paths end # Return which tcp port the launcher is using, if it's using TCP @@ -204,10 +205,6 @@ def close_binder_listeners @binder.close_listeners end - def close_binder_unix_paths - @binder.close_unix_paths - end - private # If configured, write the pid of the current process out diff --git a/lib/puma/single.rb b/lib/puma/single.rb index 1b5a56e760..a1673a8eb7 100644 --- a/lib/puma/single.rb +++ b/lib/puma/single.rb @@ -118,7 +118,6 @@ def run rescue Interrupt # Swallow it end - @launcher.close_binder_unix_paths end end end From 9ce8d4fc4ee24a65fc96838d604235d521feaf06 Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Tue, 1 Oct 2019 09:01:45 -0500 Subject: [PATCH 2/4] Add tests for pre-existing unix paths 1. test_binder.rb - test_pre_existing_unix 2. test_integration_cluster.rb - test_pre_existing_unix --- test/test_binder.rb | 17 +++++++++++++++++ test/test_integration_cluster.rb | 20 +++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/test/test_binder.rb b/test/test_binder.rb index dc45adfbfe..198370eca5 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -78,6 +78,23 @@ def test_allows_both_tcp_and_unix assert_parsing_logs_uri [:tcp, :unix] end + def test_pre_existing_unix + skip UNIX_SKT_MSG unless UNIX_SKT_EXIST + unix_path = "test/#{name}_server.sock" + + File.open(unix_path, mode: 'wb') { |f| f.puts 'pre eixisting' } + @binder.parse ["unix://#{unix_path}"], @events + + assert_match %r!unix://#{unix_path}!, @events.stdout.string + + refute_includes @binder.instance_variable_get(:@unix_paths), unix_path + + ensure + if UNIX_SKT_EXIST + File.unlink unix_path if File.exist? unix_path + end + end + private def assert_parsing_logs_uri(order = [:unix, :tcp]) diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index f6634c3b6a..0973b5112f 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -12,7 +12,25 @@ def setup end def teardown - super if HAS_FORK + return if skipped? + super + end + + def test_pre_existing_unix + skip UNIX_SKT_MSG unless UNIX_SKT_EXIST + + File.open(@bind_path, mode: 'wb') { |f| f.puts 'pre eixisting' } + + cli_server "-w #{WORKERS} -t 0:6 -q test/rackup/sleep_step.ru", unix: :unix + + stop_server + + assert File.exist?(@bind_path) + + ensure + if UNIX_SKT_EXIST + File.unlink @bind_path if File.exist? @bind_path + end end def test_siginfo_thread_print From 7b228da3f66719bf0ebedd13809ea1a3cb22841b Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Tue, 1 Oct 2019 10:34:54 -0500 Subject: [PATCH 3/4] Update History.md --- History.md | 1 + 1 file changed, 1 insertion(+) diff --git a/History.md b/History.md index 632ac5fd54..efaf3375a2 100644 --- a/History.md +++ b/History.md @@ -4,6 +4,7 @@ * Your feature goes here (#Github Number) * Bugfixes + * Fix handling of pre-existing/systemd unix binder files (#1842, #1988) * Deal with multiple calls to bind correctly (#1986, #1994, #2006) ## 4.2.0 / 2019-09-23 From 8ad26eb66297a9795418f8a28b1230c664bae12d Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Tue, 1 Oct 2019 11:28:33 -0500 Subject: [PATCH 4/4] review updates --- History.md | 2 +- test/test_binder.rb | 6 +++++- test/test_integration_cluster.rb | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/History.md b/History.md index efaf3375a2..468cb401e2 100644 --- a/History.md +++ b/History.md @@ -4,7 +4,7 @@ * Your feature goes here (#Github Number) * Bugfixes - * Fix handling of pre-existing/systemd unix binder files (#1842, #1988) + * Fix socket activation of systemd (pre-existing) unix binder files (#1842, #1988) * Deal with multiple calls to bind correctly (#1986, #1994, #2006) ## 4.2.0 / 2019-09-23 diff --git a/test/test_binder.rb b/test/test_binder.rb index 198370eca5..5d16120076 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -82,13 +82,17 @@ def test_pre_existing_unix skip UNIX_SKT_MSG unless UNIX_SKT_EXIST unix_path = "test/#{name}_server.sock" - File.open(unix_path, mode: 'wb') { |f| f.puts 'pre eixisting' } + File.open(unix_path, mode: 'wb') { |f| f.puts 'pre existing' } @binder.parse ["unix://#{unix_path}"], @events assert_match %r!unix://#{unix_path}!, @events.stdout.string refute_includes @binder.instance_variable_get(:@unix_paths), unix_path + @binder.close_unix_paths + + assert File.exist?(unix_path) + ensure if UNIX_SKT_EXIST File.unlink unix_path if File.exist? unix_path diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index 0973b5112f..9679f274fb 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -19,9 +19,9 @@ def teardown def test_pre_existing_unix skip UNIX_SKT_MSG unless UNIX_SKT_EXIST - File.open(@bind_path, mode: 'wb') { |f| f.puts 'pre eixisting' } + File.open(@bind_path, mode: 'wb') { |f| f.puts 'pre existing' } - cli_server "-w #{WORKERS} -t 0:6 -q test/rackup/sleep_step.ru", unix: :unix + cli_server "-w #{WORKERS} -q test/rackup/sleep_step.ru", unix: :unix stop_server