Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #369 thread issue when calling translate with fallbacks #374

Merged
merged 1 commit into from Jul 6, 2017
Merged

Fixes #369 thread issue when calling translate with fallbacks #374

merged 1 commit into from Jul 6, 2017

Conversation

chadrschroeder
Copy link

This pull request is a fix for the thread issues discuss in #369.

In Version 0.7.0 the I18n::Backend::Fallbacks#translate method had an internal option called fallback. This was set to true when the fallback logic was in progress and was used as a way to prevent fallbacks from getting stuck in an infinite loop:

def translate(locale, key, options = {})
  return super if options[:fallback]
  ...
  options[:fallback] = true
  I18n.fallbacks[locale].each do |fallback|
    ...
  end
  options.delete(:fallback)

Although fallback was an internal option, this wiki made reference to a fallbacks flag that could be used to disable fallbacks.

Unfortunately the fallbacks flag didn't do anything so pull request #354 made a change to allow for passing fallback: false as a way to disable the fallback logic. When that was done this commit added an instance variable called @fallback_locked which breaks thread-safety:

def translate(locale, key, options = {})
  return super unless options.fetch(:fallback, true)

  # Depending on the timing, a given thread can incorrectly short-circuit out of the 
  # fallback flow on the next line if @fallback_locked has been set to true but another 
  # thread
  return super if (@fallback_locked ||= false)  # <---
  ...

  begin
    @fallback_locked = true
     I18n.fallbacks[locale].each do |fallback|
  ...

  ensure
    @fallback_locked = false

This pull request will remove the instance variable and revert back to the local options hash, using a new internal option called :fallback_in_progress to control the fallback looping structure in a way that is thread-safe.

@radar
Copy link
Collaborator

radar commented Jul 6, 2017

Thank you for taking the time to build this PR and to write up such a great description of the bug it fixes. I think this looks good, so I'm going to go ahead and merge it.

@radar radar merged commit dd6ef4a into ruby-i18n:master Jul 6, 2017
@radar
Copy link
Collaborator

radar commented Jul 6, 2017

I will try to do a 0.8.5 release tomorrow morning. I would do it now but I am currently on a train which has poor internet quality.

@radar
Copy link
Collaborator

radar commented Jul 8, 2017

0.8.5 has been released with this patch included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants