faker: Locale setting can be ignored

The bug

Depending on how locale is set, it may be ignored in threaded server environments.

In particular, we use Faker at runtime in a deployed environment (using Puma) where a user can press a button and the server generates them fake data in a form.

To Reproduce

  1. Set up a Rails app with a threaded server (eg Puma)
  2. In a lib file, or an initializer, add a locale setting line: Faker::Config.locale = :'en-GB'
  3. Make a request to the server for fake data.
  4. You will receive fake data for the default locale :en, which is American (eg Zip codes) not :en-GB (eg Postcodes)

Cause

The cause is the new threaded safety: https://github.com/faker-ruby/faker/pull/2520 introduced in Faker 2.23.0.

Puma works by preloading the app, then process forking it (and also maybe generating new server threads). From the above reproduction steps, you can see that the Faker locale will be set in the ThreadLocal of the master process Thread.current[:faker_config_locale]. However, all the threaded child processes do not inherit the ThreadLocal value, so it is blank, and defaults to :en.

Reverting to Faker 2.22.0 solves the issue.

Possible solution

When the locale is set for the first time, it could become the new default instead of the logic here: https://github.com/faker-ruby/faker/blob/master/lib/faker.rb#L23

Open to other ideas!

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 32 (25 by maintainers)

Commits related to this issue

Most upvoted comments

@stefannibrasil More like a month rather than a week.

@stefannibrasil Will do next week!

I’m pretty sure the only mistake in #2520 was removing the locale class variable. Instead, it should have added the fiber-local as the new, first source of truth and fell back to @locale if that fiber local is not set.

In essence,

def locale
  Thread.current[:faker_config_locale] || @locale || (I18n.available_locales.include?(I18n.locale) ? I18n.locale : I18n.available_locales.first)
end

As for setting the locale, that’s a bit trickier. I don’t understand the desired behavior, but I think what you want is to set a “default” locale, and then be able to set the locale again on a per-thread basis. So you probably want a configuration option which sets the default locale in a non-threadsafe way (i.e. changing the @locale class variable) and then an API which sets the locale for the current thread.

Hey, @stefannibrasil sure thing! Sorry for the late response. Kind of got lost in work. But now I have time!

Again, open to any input and other info.

Also, concerning the whole idea of being able to configurate faker using something like:

Faker.configuration do |config|
end

I was able to make it work, but it’s definitely a breaking change, even for the tests. At least the way I found on how to do it, which would require making the Config module a class. I’m still going to look into it and se if it can’t be done with a module in a backwards compatible way.

Thanks for looking at this @mateusdeap

Puma can serve multiple requests in two ways (and it is recommended to use both).

  1. Process forking. Basically Rails “starts”, then multiple processes are forked off which are all (initially) replicas. They no longer alter each others memory after that. If you add something to an in-memory cache, or change a global variable, the others will not be aware of it.
  2. Threading. In each process (above) Puma can spin up multiple threads. These are all in the same process, and can talk to each other and share memory. Global variable changes are shared.

More discussion about it is here: https://stackoverflow.com/questions/24280743/what-is-the-difference-between-workers-and-threads-in-puma.

I’d say the simplest solution may be for there to be a non-Threadsafe “default locale” which can be set during Rails startup (before any forking). Maybe then you could (in tests) set a thread local one… And the “lookup” code internally in faker would check the Thread local one, then if not found fall back to the global one?

A +1 from us at https://github.com/DFE-Digital/apply-for-teacher-training as this is preventing us from moving beyond v2.22.0 of Faker.

Our “production” use is to generate data in a sandbox environment, and to generate temporary data for ActionMailer previews.

Happy to chip in if we can be of help.

Well, if people were already using it in production, we still want to support that behavior. Besides, they could also be running tests that match closely the behavior of their apps running in production (using Puma, threads, and everything else), so in my mind, it makes sense to support that.

But are we suppose to use faker on production? It is meant to be used for development and testing ENVs. Also Ruby3.1.2 has fiber which is similar to threads, worth taking a look.

Just to update on this: I’ve made some progress, though not substantial. I’ll need to pause work for a few days, however. But I’ll be back on it in a week or so.

Sure thing!

Hey @JuanVqz I am not totally familiar with threads. But what I can gather from the above is that we want to keep the threaded behavior but we want to make sure that the locale set in the config is shared between all threads. I’ll need to learn a bit more about threads to be more useful, but does this information help?

Hey @fbuys that makes sense, I’m not familiar with threads as well.

Hey @JuanVqz I am not totally familiar with threads. But what I can gather from the above is that we want to keep the threaded behavior but we want to make sure that the locale set in the config is shared between all threads. I’ll need to learn a bit more about threads to be more useful, but does this information help?

Hey Stefanni!! I’d like to solve it but I want to understand it first, do you suggest reverting the changes in the faker.rb file from this PR will solve the issue? If so, I can do it.