-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
I18n::Backend::Memoize is not thread-safe #51
Conversation
Is there an easy way for us to solve this which does not require wrapping up each interaction in a mutex? |
None that I know of :(. We'd need Ruby version specific "memoize-friendly" hash implementation (ie. you're fine with plain Using a global hash on JRuby is basically like playing russian roulette, most of the time you'll probably be fine, however if you are unlucky you might run into a random Try it for yourself: http://gist.github.com/559673 PS: btw we're in (good?) company with Rails 3 on this one .. https://rails.lighthouseapp.com/projects/8994/tickets/5340 |
So the issue here and in the Rails 3 ticket is that we need to use ConcurrentHashMap in JRuby? Do you think you can provide a small patch? Thanks a lot! |
Replace them with ThreadSafe::Cache.
Turning this into a PR. |
Note to self: |
@thedarkone Would you be interested in updating this to use something more modern, such as concurrent-ruby? Or whatever you choose, I guess. |
Actually, I'll make an attempt at a concurrent-ruby version of this patch now. |
@thedarkone I'd like your feedback on #352 before I merge it, if you have time. |
... a sometimes reproducer for ruby-i18n#51 (needs a GIL free Ruby such as JRuby)
@thedarkone or @radar can this be closed now that #352 has been merged or are you looking for a PR for something else to complete it? |
Yes this can now be closed. |
Backend::Memoize
uses a plainHash
that is shared between all threads, this is obviously not thread-safe and the module can't be used in truly threaded environments such as JRuby.