Properly close inherited servers on fork

I noticed some UNIXServer and AppServer accumulating with
reforking enabled.

The reason is that the `at_exit { stop }` make `Server` instances
immortal, so they are never GC, and their open socket accumulate and
are never closed.

That callback is useless anyway, as on exit the finalizer of these
objects will be invoked, and they'll cleanup after themselves.

lib/app_profiler/server.rb

@@ -198,11 +198,26 @@ def stop
       end
 
       class UNIX < Transport
+        class << self
+          def unlink_socket(path, pid)
+            ->(_) do
+              if Process.pid == pid && File.exist?(path)
+                begin
+                  File.unlink(path)
+                rescue SystemCallError
+                  # Let not raise in a finalizer
+                end
+              end
+            end
+          end
+        end
+
         def start
           FileUtils.mkdir_p(PROFILER_TEMPFILE_PATH)
           @socket_file = File.join(PROFILER_TEMPFILE_PATH, "app-profiler-#{Process.pid}.sock")
           File.unlink(@socket_file) if File.exist?(@socket_file) && File.socket?(@socket_file)
           @socket = UNIXServer.new(@socket_file)
+          ObjectSpace.define_finalizer(self, self.class.unlink_socket(@socket_file, Process.pid))
         end
 
         def client
@@ -213,6 +228,10 @@ def stop
           File.unlink(@socket_file) if File.exist?(@socket_file) && File.socket?(@socket_file)
           @socket.close
         end
+
+        def abandon
+          @socket.close
+        end
       end
 
       class TCP < Transport
@@ -238,6 +257,11 @@ def stop
           @port_file.unlink
           @socket.close
         end
+
+        def abandon
+          @port_file.close # NB: Tempfile finalizer checks Process.pid to avoid unlinking inherited IOs.
+          @socket.close
+        end
       end
 
       def initialize(transport, logger)
@@ -257,7 +281,6 @@ def initialize(transport, logger)
           "[AppProfiler::Server] listening on addr=#{@transport.socket.addr}"
         )
         @pid = Process.pid
-        at_exit { stop }
       end
 
       def client
@@ -324,10 +347,12 @@ def serve
       end
 
       def stop
-        return unless @pid == Process.pid
-
         @listen_thread.kill
-        @transport.stop
+        if @pid == Process.pid
+          @transport.stop
+        else
+          @transport.abandon
+        end
       end
     end
 
@@ -371,7 +396,10 @@ def stop
       private
 
       def profile_server
-        @profile_server = nil if @pid != Process.pid
+        if @pid != Process.pid
+          @profile_server&.stop
+          @profile_server = nil
+        end
         @profile_server
       end
 

test/app_profiler/profile_server_test.rb

@@ -82,6 +82,42 @@ class UNIXServerTest < ProfileServerTestCase
       ensure
         assert_nil(Server.stop)
       end
+
+
+      Transport = AppProfiler::Server.const_get(:ProfileServer)::Transport
+      test "servers are abandoned on fork" do
+        server = AppProfiler::Server.start(@logger)
+        open_servers = ObjectSpace.each_object(Transport).select { |t| !t.socket.closed? }
+        assert_equal 1, open_servers.size
+
+        r, w = IO.pipe
+        pid = fork do
+          open_servers = ObjectSpace.each_object(Transport).select { |t| !t.socket.closed? }
+          w.write(Marshal.dump(open_servers.size))
+
+          AppProfiler::Server.start(@logger)
+
+          open_servers = ObjectSpace.each_object(Transport).select { |t| !t.socket.closed? }
+          w.write(Marshal.dump(open_servers.size))
+          Marshal.load(r)
+        end
+        assert_equal 1, Marshal.load(r)
+        assert_equal 1, Marshal.load(r)
+
+        assert_equal true, File.exist?(File.join(ProfileServer::PROFILER_TEMPFILE_PATH, "app-profiler-#{Process.pid}.sock"))
+        assert_equal true, File.exist?(File.join(ProfileServer::PROFILER_TEMPFILE_PATH, "app-profiler-#{pid}.sock"))
+
+        w.write(Marshal.dump(:done))
+        w.close
+
+        _, status = Process.wait2(pid)
+        assert_predicate status, :success?
+
+        assert_equal true, File.exist?(File.join(ProfileServer::PROFILER_TEMPFILE_PATH, "app-profiler-#{Process.pid}.sock"))
+        assert_equal false, File.exist?(File.join(ProfileServer::PROFILER_TEMPFILE_PATH, "app-profiler-#{pid}.sock"))
+      ensure
+        server.stop
+      end
     end
 
     class ProfileServerTest < ProfileServerTestCase