Ensure pipe is closed after test run

`Forking.run_in_isolation` opens two ends of a pipe. The fork process closes
the read end, writes to it, and then terminates (which presumably closes the
file descriptors on its end). The parent process closes the write end, reads
from it, and returns, never closing the read end.

This results in an accumulation of open file descriptors, which can cause
errors if the limit is reached.

One approach to fixing this would be to simply close the read end of the pipe
in the parent process. However, it is more idiomatic to open the pipe given a
block, which automatically closes the pipe after the block exits.

Author: sambostock

activesupport/CHANGELOG.md

@@ -1,3 +1,16 @@
+*   Ensure `ActiveSupport::Testing::Isolation::Forking` closes pipes
+
+    Previously, `Forking.run_in_isolation` opened two ends of a pipe. The fork
+    process closed the read end, wrote to it, and then terminated (which
+    presumably closed the file descriptors on its end). The parent process
+    closed the write end, read from it, and returned, never closing the read
+    end.
+
+    This resulted in an accumulation of open file descriptors, which could
+    cause errors if the limit is reached.
+
+    *Sam Bostock*
+
 *   Fix `Time#change` and `Time#advance` for times around the end of Daylight
     Saving Time.
 

activesupport/lib/active_support/testing/isolation.rb

@@ -25,38 +25,39 @@ def run
 
       module Forking
         def run_in_isolation(&blk)
-          read, write = IO.pipe
-          read.binmode
-          write.binmode
+          IO.pipe do |read, write|
+            read.binmode
+            write.binmode
 
-          pid = fork do
-            read.close
-            yield
-            begin
-              if error?
-                failures.map! { |e|
-                  begin
-                    Marshal.dump e
-                    e
-                  rescue TypeError
-                    ex = Exception.new e.message
-                    ex.set_backtrace e.backtrace
-                    Minitest::UnexpectedError.new ex
-                  end
-                }
+            pid = fork do
+              read.close
+              yield
+              begin
+                if error?
+                  failures.map! { |e|
+                    begin
+                      Marshal.dump e
+                      e
+                    rescue TypeError
+                      ex = Exception.new e.message
+                      ex.set_backtrace e.backtrace
+                      Minitest::UnexpectedError.new ex
+                    end
+                  }
+                end
+                test_result = defined?(Minitest::Result) ? Minitest::Result.from(self) : dup
+                result = Marshal.dump(test_result)
               end
-              test_result = defined?(Minitest::Result) ? Minitest::Result.from(self) : dup
-              result = Marshal.dump(test_result)
+
+              write.puts [result].pack("m")
+              exit!
             end
 
-            write.puts [result].pack("m")
-            exit!
+            write.close
+            result = read.read
+            Process.wait2(pid)
+            result.unpack1("m")
           end
-
-          write.close
-          result = read.read
-          Process.wait2(pid)
-          result.unpack1("m")
         end
       end