From 3163932f70e824906d48edc9665b4017a4669797 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Sun, 25 Jul 2010 19:04:44 +0200 Subject: Fix #4244 - Cached Attributes is not thread safe The underlying hash is not protected and thus two threads accessing the cached value at the same time and one expiring the value can result in a race condition. This patch synchronizes the access to the value_cache underlying hash. Signed-off-by: Brice Figureau --- lib/puppet/util/cacher.rb | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/puppet/util/cacher.rb b/lib/puppet/util/cacher.rb index 8785c694f..3dddec0d4 100644 --- a/lib/puppet/util/cacher.rb +++ b/lib/puppet/util/cacher.rb @@ -1,3 +1,5 @@ +require 'monitor' + module Puppet::Util::Cacher module Expirer attr_reader :timestamp @@ -49,7 +51,7 @@ module Puppet::Util::Cacher define_method(name.to_s + "=") do |value| # Make sure the cache timestamp is set cache_timestamp - value_cache[name] = value + value_cache.synchronize { value_cache[name] = value } end if ttl = options[:ttl] @@ -70,6 +72,7 @@ module Puppet::Util::Cacher # Methods that get added to instances. module InstanceMethods + def expire # Only expire if we have an expirer. This is # mostly so that we can comfortably handle cases @@ -92,15 +95,17 @@ module Puppet::Util::Cacher end def cached_value(name) - # Allow a nil expirer, in which case we regenerate the value every time. - if expired_by_expirer?(name) - value_cache.clear - @cache_timestamp = Time.now - elsif expired_by_ttl?(name) - value_cache.delete(name) + value_cache.synchronize do + # Allow a nil expirer, in which case we regenerate the value every time. + if expired_by_expirer?(name) + value_cache.clear + @cache_timestamp = Time.now + elsif expired_by_ttl?(name) + value_cache.delete(name) + end + value_cache[name] = send("init_#{name}") unless value_cache.include?(name) + value_cache[name] end - value_cache[name] = send("init_#{name}") unless value_cache.include?(name) - value_cache[name] end def expired_by_expirer?(name) @@ -121,7 +126,7 @@ module Puppet::Util::Cacher end def value_cache - @value_cache ||= {} + @value_cache ||= {}.extend(MonitorMixin) end end end -- cgit