Skip to content

Commit

Permalink
Make cache and values fully thread-safe
Browse files Browse the repository at this point in the history
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 <[email protected]>
resolves #8951
  • Loading branch information
joshcooper committed Nov 16, 2023
1 parent d3caef9 commit f890191
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions lib/puppet/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'forwardable'
require 'fileutils'
require 'concurrent'
require_relative 'concurrent/lock'

# The class for handling configuration files.
class Puppet::Settings
Expand Down Expand Up @@ -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 = []
Expand Down

0 comments on commit f890191

Please sign in to comment.