[Experimental] Pitchfork::Info.close_all_fds!

Ensuring that an application is fork safe can be complex, and
in case of mistake, one of the error condition is shared file
descriptors, causing one worker to read packets from another
which could have dire consequences.

Rather than to track down all the file descriptors that need
reopening, `close_all_fds!` allows to automatically close all
file descriptors that aren't explicitly marked as needing to
be kept.

And in case of mistake, the error condition is that trying to
read or write into that file descriptor will fail, which is
much less critical than to read packets from another worker.

lib/pitchfork.rb

@@ -151,6 +151,7 @@ def self.clean_fork(setpgid: true, &block)
         while block
           block = catch(self) do
             Thread.current[:pitchfork_handle_clean_fork] = true
+            Info.close_all_fds!
             block.call
             nil
           end

lib/pitchfork/flock.rb

@@ -19,6 +19,10 @@ def at_fork
       nil
     end
 
+    def to_io
+      @file
+    end
+
     def unlink
       File.unlink(@file.path)
     rescue Errno::ENOENT

lib/pitchfork/http_server.rb

@@ -129,6 +129,7 @@ def initialize(app, options = {})
       @respawn = false
       @last_check = Pitchfork.time_now
       @promotion_lock = Flock.new("pitchfork-promotion")
+      Info.keep_io(@promotion_lock)
 
       options = options.dup
       @ready_pipe = options.delete(:ready_pipe)
@@ -180,6 +181,7 @@ def start(sync = true)
       # It's also used by newly spawned children to send their soft_signal pipe
       # to the master when they are spawned.
       @control_socket.replace(Pitchfork.socketpair)
+      Info.keep_ios(@control_socket)
       @master_pid = $$
 
       # setup signal handlers before writing pid file in case people get
@@ -264,6 +266,7 @@ def listen(address, opt = {}.merge(listener_opts[address] || {}))
           io = server_cast(io)
         end
         logger.info "listening on addr=#{sock_name(io)} fd=#{io.fileno}"
+        Info.keep_io(io)
         LISTENERS << io
         io
       rescue Errno::EADDRINUSE => err

lib/pitchfork/info.rb

@@ -6,10 +6,40 @@ module Pitchfork
   module Info
     @workers_count = 0
     @fork_safe = true
+    @kept_ios = ObjectSpace::WeakMap.new
 
     class << self
       attr_accessor :workers_count
 
+      def keep_io(io)
+        @kept_ios[io] = io if io && !io.to_io.closed?
+        io
+      end
+
+      def keep_ios(ios)
+        ios.each { |io| keep_io(io) }
+      end
+
+      def close_all_fds!
+        ignored_fds = [$stdin.to_i, $stdout.to_i, $stderr.to_i]
+        @kept_ios.each_value do |io_like|
+          if io = io_like&.to_io
+            ignored_fds << io.to_i unless io.closed?
+          end
+        end
+
+        all_fds = Dir.children("/dev/fd").map(&:to_i)
+        all_fds -= ignored_fds
+
+        all_fds.each do |fd|
+          IO.for_fd(fd).close
+        rescue ArgumentError
+          # RubyVM internal file descriptor, leave it alone
+        rescue Errno::EBADF
+          # Likely a race condition
+        end
+      end
+
       def fork_safe?
         @fork_safe
       end

lib/pitchfork/worker.rb

@@ -198,7 +198,7 @@ def close # :nodoc:
     end
 
     def create_socketpair!
-      @to_io, @master = Pitchfork.socketpair
+      @to_io, @master = Info.keep_ios(Pitchfork.socketpair)
     end
 
     def after_fork_in_child
@@ -208,6 +208,7 @@ def after_fork_in_child
     private
 
     def pipe=(socket)
+      Info.keep_io(socket)
       @master = MessageSocket.new(socket)
     end
 

test/slow/test_server.rb

@@ -80,6 +80,7 @@ class WebServerStartTest < Pitchfork::Test
 
   def test_preload_app
     tmp = Tempfile.new('test_preload_app_config')
+    Pitchfork::Info.keep_io(tmp)
     ObjectSpace.undefine_finalizer(tmp)
     app = lambda { ||
       tmp.sysseek(0)

test/slow/test_signals.rb

@@ -25,8 +25,8 @@ def setup
     @bs = 1 * 1024 * 1024
     @count = 100
     @port = unused_port
-    @sock = Tempfile.new('pitchfork.sock')
-    @tmp = Tempfile.new('pitchfork.write')
+    @sock = Pitchfork::Info.keep_io(Tempfile.new('pitchfork.sock'))
+    @tmp = Pitchfork::Info.keep_io(Tempfile.new('pitchfork.write'))
     @tmp.sync = true
     File.unlink(@sock.path)
     File.unlink(@tmp.path)
@@ -75,7 +75,7 @@ def test_worker_dies_on_dead_master
   end
 
   def test_sleepy_kill
-    rd, wr = IO.pipe
+    rd, wr = Pitchfork::Info.keep_ios(IO.pipe)
     pid = fork {
       rd.close
       app = lambda { |env| wr.syswrite('.'); sleep; [ 200, {}, [] ] }

test/slow/test_upload.rb

@@ -143,7 +143,7 @@ def test_put_keepalive_truncates_small_overwrite
   end
 
   def test_put_excessive_overwrite_closed
-    tmp = Tempfile.new('overwrite_check')
+    tmp = Pitchfork::Info.keep_io(Tempfile.new('overwrite_check'))
     tmp.sync = true
     start_server(lambda { |env|
       nr = 0

test/unit/test_ccc.rb

@@ -6,9 +6,9 @@ def test_ccc_tcpi
     host = '127.0.0.1'
     port = unused_port
 
-    rd, wr = IO.pipe
-    sleep_pipe = IO.pipe
-    ready_read, ready_write = IO.pipe
+    rd, wr = Pitchfork::Info.keep_ios(IO.pipe)
+    sleep_pipe = Pitchfork::Info.keep_ios(IO.pipe)
+    ready_read, ready_write = Pitchfork::Info.keep_ios(IO.pipe)
 
     pid = fork do
       ready_read.close