From f89019189cee62a8c3df74d960432569efcd8859 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Thu, 16 Nov 2023 14:01:03 -0800 Subject: [PATCH] Make cache and values fully thread-safe Not locking the default initialization can lead to race-conditions. I don't think we can switch to Concurrent::Map, and it's compute_if_absent method, because insertion order won't be maintained. So synchronize the long way. ref: ruby-concurrency/concurrent-ruby#970 Co-authored-by: Maciej Mensfeld resolves #8951 --- lib/puppet/settings.rb | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/puppet/settings.rb b/lib/puppet/settings.rb index 26d85592c70..d7cac4b9920 100644 --- a/lib/puppet/settings.rb +++ b/lib/puppet/settings.rb @@ -5,6 +5,7 @@ require 'forwardable' require 'fileutils' require 'concurrent' +require_relative 'concurrent/lock' # The class for handling configuration files. class Puppet::Settings @@ -146,8 +147,21 @@ def initialize @configuration_file = nil # And keep a per-environment cache - @cache = Concurrent::Hash.new { |hash, key| hash[key] = Concurrent::Hash.new } - @values = Concurrent::Hash.new { |hash, key| hash[key] = Concurrent::Hash.new } + # We can't use Concurrent::Map because we want to preserve insertion order + @cache_lock = Puppet::Concurrent::Lock.new + @cache = Concurrent::Hash.new do |hash, key| + @cache_lock.synchronize do + break hash[key] if hash.key?(key) + hash[key] = Concurrent::Hash.new + end + end + @values_lock = Puppet::Concurrent::Lock.new + @values = Concurrent::Hash.new do |hash, key| + @values_lock.synchronize do + break hash[key] if hash.key?(key) + hash[key] = Concurrent::Hash.new + end + end # The list of sections we've used. @used = []