Merge pull request #20408 from Homebrew/fd-leak-fix

os/linux/elf: fix file descriptor leak

Library/Homebrew/os/linux/elf.rb

@@ -52,7 +52,6 @@ module ELFShim
     @interpreter = T.let(nil, T.nilable(String))
     @dynamic_elf = T.let(nil, T.nilable(T::Boolean))
     @metadata = T.let(nil, T.nilable(Metadata))
-    @patchelf_patcher = nil
 
     super
   end
@@ -123,7 +122,7 @@ module ELFShim
   # "/lib:/usr/lib:/usr/local/lib"
   sig { returns(T.nilable(String)) }
   def rpath
-    @rpath ||= rpath_using_patchelf_rb
+    metadata.rpath
   end
 
   # An array of runtime search path entries, such as:
@@ -134,7 +133,7 @@ module ELFShim
 
   sig { returns(T.nilable(String)) }
   def interpreter
-    @interpreter ||= patchelf_patcher.interpreter
+    metadata.interpreter
   end
 
   def patch!(interpreter: nil, rpath: nil)
@@ -149,11 +148,12 @@ module ELFShim
 
   sig { returns(T::Boolean) }
   def dynamic_elf?
-    @dynamic_elf ||= elf_parser.segment_by_type(:DYNAMIC).present?
+    metadata.dynamic_elf?
   end
 
+  sig { returns(T::Array[String]) }
   def section_names
-    @section_names ||= elf_parser.sections.map(&:name).compact_blank
+    metadata.section_names
   end
 
   # Helper class for reading metadata from an ELF file.
@@ -164,35 +164,53 @@ module ELFShim
     sig { returns(T.nilable(String)) }
     attr_reader :dylib_id
 
+    sig { returns(T::Boolean) }
+    def dynamic_elf?
+      @dynamic_elf
+    end
+
+    sig { returns(T.nilable(String)) }
+    attr_reader :interpreter
+
+    sig { returns(T.nilable(String)) }
+    attr_reader :rpath
+
     sig { returns(T::Array[String]) }
-    attr_reader :dylibs
+    attr_reader :section_names
 
     sig { params(path: ELFShim).void }
     def initialize(path)
-      @path = T.let(path, ELFShim)
-      @dylibs = T.let([], T::Array[String])
-      @dylib_id = T.let(nil, T.nilable(String))
-      @dylib_id, needed = needed_libraries path
-      @dylibs = needed.map { |lib| find_full_lib_path(lib).to_s } if needed.present?
+      require "patchelf"
+      patcher = path.patchelf_patcher
 
-      @metadata = T.let(nil, T.nilable(T::Hash[String, T.untyped]))
+      @path = T.let(path, ELFShim)
+      @dylibs = T.let(nil, T.nilable(T::Array[String]))
+      @dylib_id = T.let(nil, T.nilable(String))
+
+      dynamic_segment = patcher.elf.segment_by_type(:dynamic)
+      @dynamic_elf = dynamic_segment.present?
+      @dylib_id, @needed = if @dynamic_elf
+        [patcher.soname, patcher.needed]
+      else
+        [nil, []]
+      end
+
+      @interpreter = patcher.interpreter
+      @rpath = patcher.runpath || patcher.rpath
+      @section_names = patcher.elf.sections.map(&:name).compact_blank
+
+      @dt_flags_1 = dynamic_segment&.tag_by_type(:flags_1)&.value
+    end
+
+    sig { returns(T::Array[String]) }
+    def dylibs
+      @dylibs ||= @needed.map { |lib| find_full_lib_path(lib).to_s }
     end
 
     private
 
-    def needed_libraries(path)
-      return [nil, []] unless path.dynamic_elf?
-
-      needed_libraries_using_patchelf_rb path
-    end
-
-    def needed_libraries_using_patchelf_rb(path)
-      patcher = path.patchelf_patcher
-      [patcher.soname, patcher.needed]
-    end
-
     def find_full_lib_path(basename)
-      local_paths = (path.patchelf_patcher.runpath || path.patchelf_patcher.rpath)&.split(":")
+      local_paths = rpath&.split(":")
 
       # Search for dependencies in the runpath/rpath first
       local_paths&.each do |local_path|
@@ -202,11 +220,10 @@ module ELFShim
       end
 
       # Check if DF_1_NODEFLIB is set
-      dt_flags_1 = path.elf_parser.segment_by_type(:dynamic)&.tag_by_type(:flags_1)
-      nodeflib_flag = if dt_flags_1.nil?
+      nodeflib_flag = if @dt_flags_1.nil?
         false
       else
-        dt_flags_1.value & ELFTools::Constants::DF::DF_1_NODEFLIB != 0
+        @dt_flags_1 & ELFTools::Constants::DF::DF_1_NODEFLIB != 0
       end
 
       linker_library_paths = OS::Linux::Ld.library_paths
@@ -247,13 +264,12 @@ module ELFShim
     patcher.save(patchelf_compatible: true)
   end
 
-  def rpath_using_patchelf_rb
-    patchelf_patcher.runpath || patchelf_patcher.rpath
-  end
-
+  # Don't cache the patcher; it keeps the ELF file open so long as it is alive.
+  # Instead, for read-only access to the ELF file's metadata, fetch it and cache
+  # it with {Metadata}.
   def patchelf_patcher
     require "patchelf"
-    @patchelf_patcher ||= ::PatchELF::Patcher.new to_s, on_error: :silent
+    ::PatchELF::Patcher.new to_s, on_error: :silent
   end
 
   sig { returns(Metadata) }