Merge pull request #42 from ekohl/really-fix-file-closing

Avoid closing directory we're iterating

Author: nmburgan

lib/puppet/util.rb

@@ -478,13 +478,15 @@ def safe_posix_fork(stdin = $stdin, stdout = $stdout, stderr = $stderr, &block)
       $stderr = STDERR
 
       begin
-        Dir.foreach('/proc/self/fd') do |f|
-          if %{^\d+$}.match?(f) && f.to_i >= 3
-            begin
-              IO.new(f.to_i).close
-            rescue
-              nil
-            end
+        d = Dir.new('/proc/self/fd')
+        ignore_fds = ['.', '..', d.fileno.to_s]
+        d.each_child do |f|
+          next if ignore_fds.include?(f) || f.to_i < 3
+
+          begin
+            IO.new(f.to_i).close
+          rescue
+            nil
           end
         end
       rescue Errno::ENOENT, Errno::ENOTDIR # /proc/self/fd not found, /proc/self not a dir

spec/unit/util_spec.rb

@@ -616,18 +616,20 @@ def withenv_utf8(&block)
       fds.each do |fd|
         if fd == '.' || fd == '..'
           next
-        elsif ['0', '1', '2'].include? fd
+        elsif ['0', '1', '2', '5'].include? fd
           expect(IO).not_to receive(:new).with(fd.to_i)
         else
           expect(IO).to receive(:new).with(fd.to_i).and_return(double('io', close: nil))
         end
       end
 
-      dir_expectation = receive(:foreach).with('/proc/self/fd')
+      dir = double(Dir, fileno: '5')
+      dir_expectation = receive(:each_child)
       fds.each do |fd|
         dir_expectation = dir_expectation.and_yield(fd)
       end
-      allow(Dir).to dir_expectation
+      allow(dir).to dir_expectation
+      allow(Dir).to receive(:new).with('/proc/self/fd').and_return(dir)
       Puppet::Util.safe_posix_fork
     end
 
@@ -636,7 +638,7 @@ def withenv_utf8(&block)
       # letting it actually close fds, which seems risky
       (0..2).each {|n| expect(IO).not_to receive(:new).with(n)}
       (3..256).each {|n| expect(IO).to receive(:new).with(n).and_return(double('io', close: nil))  }
-      allow(Dir).to receive(:foreach).with('/proc/self/fd').and_raise(Errno::ENOENT)
+      allow(Dir).to receive(:new).with('/proc/self/fd').and_raise(Errno::ENOENT)
 
       Puppet::Util.safe_posix_fork
     end
@@ -646,7 +648,7 @@ def withenv_utf8(&block)
       # letting it actually close fds, which seems risky
       (0..2).each {|n| expect(IO).not_to receive(:new).with(n)}
       (3..256).each {|n| expect(IO).to receive(:new).with(n).and_return(double('io', close: nil))  }
-      allow(Dir).to receive(:foreach).with('/proc/self/fd').and_raise(Errno::ENOTDIR)
+      allow(Dir).to receive(:new).with('/proc/self/fd').and_raise(Errno::ENOTDIR)
 
       Puppet::Util.safe_posix_fork
     end