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

Value of I18n.locale can leak between requests in a multi-threaded environment/server #381

Closed
shayonj opened this Issue Aug 27, 2017 · 10 comments

Comments

Projects
None yet
5 participants
@shayonj
Contributor

shayonj commented Aug 27, 2017

What I tried to do

I am working with a high concurrency rails app. The application serves content to requests based on the locale params or HTTP_ACCEPT_LANGUAGE header that is received specific to that request. The controller action does something like this before a new request:

    I18n.locale = params[:locale] || I18n.default_locale

However, I have witnessed requests not specific to that action return content in a locale that is not the default locale (default locale is en).

What I expected to happen

The expectation is: On a new request, if I18n.locale is not being set to anything, it should return the default locale.

Versions of i18n, rails, and anything else you think is necessary

  • i18n: 0.8.6
  • rails: 5.1.3
  • puma: 3.10.0

Replication steps

I have setup a test application here: https://github.com/shayonj/i18n-test-repro. You can replicate this issue by performing:

  • clone repository locally
  • bundle install
  • rails s (This application runs Puma with max 5 threads.)
  • On one tab open (Tab1) open http://localhost:3000?locale=es
    • Only MainController#index sets I18n.locale to params[:locale] if present one or I18n.default_locale
  • On another tab (Tab2) open http://localhost:3000/foo
    • This controller action is not tampering with I18n.locale and expects it to return en (the default locale)
  • Now, try to refresh both tabs as simultaneously/quickly as possible (simulating a high concurrency env).
  • Eventually notice, on Tab2 the current locale being rendered is no longer en but es

Here is a screen capture demonstrating the above: https://www.dropbox.com/s/zbogg3vag5at98b/i18n-locale-leak.mov?dl=0

Additional info

I can see an argument being made that every action should have something like I18n.locale = :en in a before_action block. But then I guess, thats what the point is for defining default_locale to I18n config as part of an Initializer:

  config.i18n.default_locale = :en

Proposal for a solution

As a work around, in our app, I am using a custom middleware that resets Thread.current[:i18n_config] after each request. The heart of the problem appears to be that TLS isn't cleared between requests and some how it gets persisted or shared via a Thread pool or similar implementation (common between servers like Puma/Unicorn or Passenger (ENT) with a concurrency model of threads).

That being said, the above workaround has solved the issue for us and I'd like to propose/discuss if a similar thing is possible to have in the i18n gem itself? That is, to have something like a I18n::Middleware that any rack based can utilize or plugin to their app itself (out of the box), instead of having to write/maintain one natively. Or perhaps a different solution altogether?

@radar

This comment has been minimized.

Collaborator

radar commented Oct 1, 2017

Thanks @shayonj for the excellent steps to reproduce. I did what you said to do, but I was even able to reproduce it by loading the es tab first, then going to the en tab and hitting refresh a few times. I saw that the reported locale seemed to switch "randomly" between "en" and "es". Definitely a bug there! I will investigate further.

@radar

This comment has been minimized.

Collaborator

radar commented Oct 1, 2017

Simple way to refresh fast:

for i in `seq 10`; do; curl http://localhost:3000/foo ; echo ""; done
en
es
es
es
es
en
es
es
es
es
@brightchimp

This comment has been minimized.

brightchimp commented Oct 4, 2017

We are also seeing this in production

@radar

This comment has been minimized.

Collaborator

radar commented Oct 5, 2017

Thanks for the confirmation @brightchimp. Does #382 fix this for you?

@brightchimp

This comment has been minimized.

brightchimp commented Oct 5, 2017

Hi @radar, we are just debugging at the moment and will try to test that PR next week.

@brightchimp

This comment has been minimized.

brightchimp commented Oct 10, 2017

Hi @radar - Apologies, I think our issue is related to this, but is of our own making. We explicitly set I18n.locale in a before filter. Under certain circumstances we call an earlier filter which calls root_url.

We think that because the locale is not being reset (as per this issue), the call to root_url caches the locale from the previous request in the URL options here. When rendering views with root_url it then appears as though I18n.locale is ignored.

@radar

This comment has been minimized.

Collaborator

radar commented Oct 11, 2017

Ok; no worries @brightchimp. Thanks for letting me know.

@radar

This comment has been minimized.

Collaborator

radar commented Oct 15, 2017

Closing this as I think it is fixed by #382.

@radar radar closed this Oct 15, 2017

@scmx

This comment has been minimized.

scmx commented May 25, 2018

Hi, I got this problem in development today with a very new rails 5.2 project with puma and no code that changes locale that I know of. (Unless rails-i18n automatically does that depending on request somehow?)

It seemed to pick a random locale from available_locales. I could then get a different one whenever I refreshed the page or did a curl request.

But then I found this and tried the I18n::Middleware, which seems to solve it ⭐️ .
However now I can't reproduce it after disabling the middleware.. 🤷‍♂️ 😅

@saiqulhaq

This comment has been minimized.

saiqulhaq commented Oct 31, 2018

@scmx in case you still having this issue, see rails/rails#34356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment