Skip to content

Commit

Permalink
Fix concurrency issues with file caches
Browse files Browse the repository at this point in the history
Fixes #300
  • Loading branch information
glebm committed Sep 18, 2018
1 parent 1a55ae9 commit 1bdae5f
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 19 deletions.
22 changes: 22 additions & 0 deletions lib/i18n/tasks/concurrent/cache.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

require 'i18n/tasks/concurrent/cached_value'

module I18n::Tasks::Concurrent
# A thread-safe cache.
# @since 0.9.25
class Cache
def initialize
@mutex = Mutex.new
@map = {}
end

# @param [Object] key
# @return [Object] Cached or computed value.
def fetch(key, &block)
@mutex.synchronize do
@map[key] ||= CachedValue.new(&block)
end.get
end
end
end
27 changes: 27 additions & 0 deletions lib/i18n/tasks/concurrent/cached_value.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# frozen_string_literal: true

require 'i18n/tasks/concurrent/cached_value'

module I18n::Tasks::Concurrent
# A thread-safe memoized value.
# The given computation is guaranteed to be invoked at most once.
# @since 0.9.25
class CachedValue
NULL = Object.new

# @param [Proc] computation The computation that returns the value to cache.
def initialize(&computation)
@computation = computation
@mutex = Mutex.new
@result = NULL
end

# @return [Object] Result of the computation.
def get
return @result unless @result == NULL
@mutex.synchronize do
@result = @computation.call
end
end
end
end
4 changes: 1 addition & 3 deletions lib/i18n/tasks/logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ def log_error(message)
end

def log_stderr(*args)
# We don't want output from different threads to get intermixed.
MUTEX.synchronize do
# 1. We don't want output from different threads to get intermixed.
# 2. StringIO is currently not thread-safe (blows up) on JRuby:
# https://github.com/jruby/jruby/issues/4417
$stderr.puts(*args)
end
end
Expand Down
10 changes: 7 additions & 3 deletions lib/i18n/tasks/scanners/files/caching_file_finder.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

require 'i18n/tasks/concurrent/cached_value'
require 'i18n/tasks/scanners/files/file_finder'

module I18n::Tasks::Scanners::Files
# Finds the files in the specified search paths with support for exclusion / inclusion patterns.
# Wraps a {FileFinder} and caches the results.
Expand All @@ -11,8 +13,7 @@ class CachingFileFinder < FileFinder
# @param (see FileFinder#initialize)
def initialize(**args)
super
@mutex = Mutex.new
@cached_paths = nil
@cached_value = ::I18n::Tasks::Concurrent::CachedValue.new { uncached_find_files }
end

# Traverse the paths and yield the matching ones.
Expand All @@ -25,10 +26,13 @@ def traverse_files
super
end

alias uncached_find_files find_files
private :uncached_find_files

# @note This method is cached, it will only access the filesystem on the first invocation.
# @return (see FileFinder#find_files)
def find_files
@cached_paths || @mutex.synchronize { @cached_paths ||= super }
@cached_value.get
end
end
end
16 changes: 7 additions & 9 deletions lib/i18n/tasks/scanners/files/caching_file_finder_provider.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require 'i18n/tasks/concurrent/cache'
require 'i18n/tasks/scanners/files/caching_file_finder'

module I18n::Tasks::Scanners::Files
Expand All @@ -10,8 +11,7 @@ module I18n::Tasks::Scanners::Files
class CachingFileFinderProvider
# @param exclude [Array<String>]
def initialize(exclude: [])
@cache = {}
@mutex = Mutex.new
@cache = ::I18n::Tasks::Concurrent::Cache.new
@defaults = { exclude: exclude }
end

Expand All @@ -20,13 +20,11 @@ def initialize(exclude: [])
# @param (see FileFinder#initialize)
# @return [CachingFileFinder]
def get(**file_finder_args)
@cache[file_finder_args] || @mutex.synchronize do
@cache[file_finder_args] ||= begin
args = file_finder_args.dup
args[:exclude] = @defaults[:exclude] + (args[:exclude] || [])
args[:exclude].uniq!
CachingFileFinder.new(**args)
end
@cache.fetch(file_finder_args) do
args = file_finder_args.dup
args[:exclude] = @defaults[:exclude] + (args[:exclude] || [])
args[:exclude].uniq!
CachingFileFinder.new(**args)
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/i18n/tasks/scanners/files/caching_file_reader.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

require 'i18n/tasks/concurrent/cache'
require 'i18n/tasks/scanners/files/file_reader'

module I18n::Tasks::Scanners::Files
# Reads the files in 'rb' mode and UTF-8 encoding.
# Wraps a {FileReader} and caches the results.
Expand All @@ -10,8 +12,7 @@ module I18n::Tasks::Scanners::Files
class CachingFileReader < FileReader
def initialize
super
@mutex = Mutex.new
@cache = {}
@cache = ::I18n::Tasks::Concurrent::Cache.new
end

# Return the contents of the file at the given path.
Expand All @@ -21,8 +22,7 @@ def initialize
# @return (see FileReader#read_file)
# @note This method is cached, it will only access the filesystem on the first invocation.
def read_file(path)
absolute_path = File.expand_path(path)
@cache[absolute_path] || @mutex.synchronize { @cache[absolute_path] ||= super }
@cache.fetch(File.expand_path(path)) { super }
end
end
end

0 comments on commit 1bdae5f

Please sign in to comment.