Fix systemd unix path handling

History.md

@@ -4,6 +4,7 @@
   * Your feature goes here (#Github Number)
 
 * Bugfixes
+  * 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

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
 

lib/puma/cluster.rb

@@ -529,7 +529,6 @@ def run
         @suicide_pipe.close
         read.close
         @wakeup.close
-        @launcher.close_binder_unix_paths
       end
     end
 

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

lib/puma/single.rb

@@ -118,7 +118,6 @@ def run
       rescue Interrupt
         # Swallow it
       end
-      @launcher.close_binder_unix_paths
     end
   end
 end

test/test_binder.rb

@@ -78,6 +78,27 @@ 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 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
+    end
+  end
+
   private
 
   def assert_parsing_logs_uri(order = [:unix, :tcp])

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 existing' }
+
+    cli_server "-w #{WORKERS} -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