File log devices opened by mixlib-log should be closed.

Ruby does not have a destructor concept so the opened files should be closed explicitely. The ruby logger which is used as a default logger
here does not close logdevices it opened until #close method is called. When #rest! happens the logger instance gets destroyed but the #close method
is not triggered.
This patch should help to solve the problem described here: https://github.com/chef/chef/issues/3435

2 files changed, 54 insertions, 0 deletions

diff --git a/lib/mixlib/log.rb b/lib/mixlib/log.rb
index 5f216f9..1d0e302 100644
--- a/lib/mixlib/log.rb
+++ b/lib/mixlib/log.rb
@@ -29,6 +29,7 @@ module Mixlib
     LEVEL_NAMES = LEVELS.invert.freeze
 
     def reset!
+      close!
       @logger, @loggers = nil, nil
     end
 
@@ -156,5 +157,26 @@ module Mixlib
       end
     end
 
+    def all_loggers
+      [@logger, *@loggers].uniq
+    end
+
+    # select all loggers with File log devices
+    def loggers_to_close
+      loggers_to_close = []
+      all_loggers.each do |logger|
+        next unless logdev = logger.instance_variable_get(:"@logdev")
+        loggers_to_close << logger if logdev.filename
+      end
+      loggers_to_close
+    end
+
+    def close!
+      # try to close all file loggers
+      loggers_to_close.each do |l|
+        l.close rescue nil
+      end
+    end
+
   end
 end
diff --git a/spec/mixlib/log_spec.rb b/spec/mixlib/log_spec.rb
index 22b3758..31695f5 100644
--- a/spec/mixlib/log_spec.rb
+++ b/spec/mixlib/log_spec.rb
@@ -146,8 +146,40 @@ describe Mixlib::Log do
 
   it "should have by default a base log level of warn" do
     logger_mock = Struct.new(:formatter, :level).new
+    expect(Logger).to receive(:new).and_return(logger_mock)
     Logit.init
     expect(Logit.level).to eq(:warn)
   end
 
+  it "should close File logger" do
+    opened_files_count_before = 0
+    ObjectSpace.each_object(File) do |f|
+      opened_files_count_before += 1 unless f.closed?
+    end
+    Logit.init('/tmp/logger.log')
+    Logit.init('/tmp/logger.log')
+    Logit.init('/tmp/logger.log')
+    opened_files_count_after = 0
+    ObjectSpace.each_object(File) do |f|
+      opened_files_count_after += 1 unless f.closed?
+    end
+    opened_files_count_after.should eq(opened_files_count_before + 1)
+  end
+
+  it "should not close IO logger" do
+    opened_files_count_before = 0
+    ObjectSpace.each_object(File) do |f|
+      opened_files_count_before += 1 unless f.closed?
+    end
+    file = File.open('/tmp/logger.log')
+    Logit.init(file)
+    Logit.init(file)
+    Logit.init(file)
+    opened_files_count_after = 0
+    ObjectSpace.each_object(File) do |f|
+      opened_files_count_after += 1 unless f.closed?
+    end
+    opened_files_count_after.should eq(opened_files_count_before + 1)
+  end
+
 end