[close #1802] Close listeners on SIGTERM

lib/puma/cluster.rb

@@ -36,16 +36,26 @@ def stop_workers
 
       begin
         if RUBY_VERSION < '2.6'
-          @workers.each { |w| Process.waitpid(w.pid) }
+          @workers.each do |w|
+            begin
+              Process.waitpid(w.pid)
+            rescue Errno::ECHILD
+              # child is already terminated
+            end
+          end
         else
           # below code is for a bug in Ruby 2.6+, above waitpid call hangs
           t_st = Process.clock_gettime(Process::CLOCK_MONOTONIC)
           pids = @workers.map(&:pid)
           loop do
             pids.reject! do |w_pid|
-              if Process.waitpid(w_pid, Process::WNOHANG)
-                log "    worker status: #{$?}"
-                true
+              begin
+                if Process.waitpid(w_pid, Process::WNOHANG)
+                  log "    worker status: #{$?}"
+                  true
+                end
+              rescue Errno::ECHILD
+                true # child is already terminated
               end
             end
             break if pids.empty?
@@ -401,6 +411,8 @@ def setup_signals
           log "Early termination of worker"
           exit! 0
         else
+          @launcher.close_binder_listeners
+
           stop_workers
           stop
 

lib/puma/launcher.rb

@@ -214,6 +214,15 @@ def restart_args
       end
     end
 
+    def close_binder_listeners
+      @binder.listeners.each do |l, io|
+        io.close
+        uri = URI.parse(l)
+        next unless uri.scheme == 'unix'
+        File.unlink("#{uri.host}#{uri.path}")
+      end
+    end
+
     private
 
     def reload_worker_directory
@@ -319,16 +328,6 @@ def prune_bundler?
       @options[:prune_bundler] && clustered? && !@options[:preload_app]
     end
 
-    def close_binder_listeners
-      @binder.listeners.each do |l, io|
-        io.close
-        uri = URI.parse(l)
-        next unless uri.scheme == 'unix'
-        File.unlink("#{uri.host}#{uri.path}")
-      end
-    end
-
-
     def generate_restart_data
       if dir = @options[:directory]
         @restart_dir = dir

test/rackup/1second.ru

@@ -0,0 +1,4 @@
+run lambda { |env|
+  sleep 1
+  [200, {}, ["Hello World"]]
+}

test/test_integration.rb

@@ -232,6 +232,58 @@ def test_restart_closes_keepalive_sockets_workers
     assert_equal "Hello World", new_reply
   end
 
+  def test_sigterm_closes_listeners_on_forked_servers
+    skip NO_FORK_MSG unless HAS_FORK
+    pid = start_forked_server("-w 2 -q test/rackup/1second.ru")
+    threads = []
+    initial_reply = nil
+    next_replies = []
+    condition_variable = ConditionVariable.new
+    mutex = Mutex.new
+
+    threads << Thread.new do
+      s = connect
+      mutex.synchronize { condition_variable.broadcast }
+      initial_reply = read_body(s)
+    end
+
+    threads << Thread.new do
+      mutex.synchronize {
+        condition_variable.wait(mutex, 1)
+        Process.kill("SIGTERM", pid)
+      }
+    end
+
+    10.times.each do |i|
+      threads << Thread.new do
+        mutex.synchronize { condition_variable.wait(mutex, 1.5) }
+
+        begin
+          s = connect
+          read_body(s)
+          next_replies << :success
+        rescue Errno::ECONNRESET
+          # connection was accepted but then closed
+          # client would see an empty response
+          next_replies << :connection_reset
+        rescue Errno::ECONNREFUSED
+          # connection was was never accepted
+          # it can therefore be re-tried before the
+          # client receives an empty reponse
+          next_replies << :connection_refused
+        end
+      end
+    end
+
+    threads.map(&:join)
+
+    assert_equal "Hello World", initial_reply
+
+    assert_includes next_replies, :connection_refused
+
+    refute_includes next_replies, :connection_reset
+  end
+
   # It does not share environments between multiple generations, which would break Dotenv
   def test_restart_restores_environment
     # jruby has a bug where setting `nil` into the ENV or `delete` do not change the