diff options
author | Max Horbul <max@gorbul.net> | 2016-01-12 13:42:01 -0800 |
---|---|---|
committer | Max Horbul <max@gorbul.net> | 2016-03-18 10:02:20 -0700 |
commit | 32b53a2632d06f3bd6e75b15a24d74b60ced1fe9 (patch) | |
tree | fc315801d7b03e2d25772d6a386c25e9f3e4db27 | |
parent | e16d10faa2821fa7c0bdebb64a5fa556354878e3 (diff) | |
download | mixlib-log-32b53a2632d06f3bd6e75b15a24d74b60ced1fe9.tar.gz |
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
-rw-r--r-- | lib/mixlib/log.rb | 22 | ||||
-rw-r--r-- | spec/mixlib/log_spec.rb | 32 |
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 |