rails: Thread safety problem in template loading: "can't add a new key into hash during iteration"
Steps to reproduce
This is a thread safety, as such it cannot be reliably reproduced.
Expected behavior
Template loading and compilation is performed lazily, even when config.eager_load = true
. Since this needs to work in production where multiple threads might try to load a template at the same time, it needs to be thread safe.
Actual behavior
Template loading is not thread safe, occasionally causing the following error:
#<ActionView::Template::Error: can't add a new key into hash during iteration>
See full trace here: https://gist.github.com/jnicklas/21b570f19c50d958f5024566dfd052f0
System configuration
Rails version: 5.0.0.beta3 (rev db9bc8097399aab9c86)
Ruby version: ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin15]
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Comments: 43 (33 by maintainers)
Commits related to this issue
- Fix template resolver cache concurrency: "can't add a new key into hash during iteration" Resolved by https://github.com/ruby-concurrency/concurrent-ruby/pull/529 Fixes #24627. — committed to rails/rails by jeremy 8 years ago
- [#close 242] Threadsafe cache loader I'm honestly still not sure how to trigger this error, however the root cause looks like two threads trying to mutate the same values from the cache. https://git... — committed to rails/sprockets by schneems 5 years ago
- [#close 242] Threadsafe cache loader I'm honestly still not sure how to trigger this error, however the root cause looks like two threads trying to mutate the same values from the cache. https://git... — committed to rails/sprockets by schneems 5 years ago
- [#close 242] Threadsafe cache loader I'm honestly still not sure how to trigger this error, however the root cause looks like two threads trying to mutate the same values from the cache. https://git... — committed to rails/sprockets by schneems 5 years ago
This issue still occurs randomly from time to time, I don’t know why but everything work fine until it happens without any reason ! it’s rare but it happens
@thedarkone & @rafaelfranca you’re absolutely right, it’s the exact same error message but a different backtrace. It’s from Sprockets, and will continue the dialog there. Need to pay a little more attention to detail before sending someone off into a code hole
@arktisklada if you are talking about the sprockets issue, it is not related to this one.
I’ve created a ticket over at concurrent-ruby ruby-concurrency/concurrent-ruby#528.
@jeremy, if you have the time, can you take a look at what I have there?
Also of note: it seems that this is a regression. This issue did not occur with earlier versions of Rails, it might be due to the switch from
ThreadSafe::Cache
toConcurrent::Map
, but the project that I’m seeing this error in tracks Rails master and it is unclear when exactly this issue started happening (yay non-determinism).Tracked this down to a separate issue with hash modification during iteration in Jbuilder. All good, @thedarkone.
@rafaelfranca thank you ❤️! I just spent an hour digging through MRI’s
hash.c
(didn’t find anything new).@arktisklada rails/sprockets#242 has nothing to do with this issue or
concurrent-ruby
. As per backtrace, are you seeing errors from sprockets or Rails?Bumped to c-r 1.0.2 @ 06dc3fba4623a77e3cc7fa04f97723462d47bf1e
c-r 1.0.2 has been released. It has the aforementioned fix.
It seems that the fix pushed to concurrent-ruby did resolve this issue for us.
@thedarkone Thank you very much for jumping on this so quickly! Your insight is always extremely valuable. I’ll continue the conversation on the c-r thread.
This shouldn’t be happening (obviously 😢).
@matthewd you are correct in assuming the iteration must be happening on a diff thread, I can’t image it to be an in issue of nested
@data[key][name][prefix][partial][locals]
look ups or anything like that.On the other hand, the iteration from the other thread should also never cause this! Since the reported issue is happening on MRI, where
C::Map
is just a glorified thin wrapper over nativeHash
, it is quite obvious that a thread iterating over a hash (e.g.Hash#each
) will trigger acan't add a new key into hash during iteration
error on all other threads attempting to insert new keys. Howeverconcurrent-ruby
is aware of that: on MRI all iterative methods ofC::Map
pre-dup
the underlyingHash
when iterating (this is obviously inefficient, especially for large maps, but we more than make up for this by fully relying on MRI’s C-basedHash
).I also just torture tested iteration with insertion on MRI and couldn’t reproduce 😿.
Concurrent::Map
is a copy-pasted and renamedThreadSafe::Cache
. Additionally, there were almost no changes to the code in the last 3-4 years, so this shouldn’t be a version thing either.@jnicklas can you make sure you’re not using some code that perhaps pries open
Concurrent::Map
accessing its underlying@backend
i-var? You could maybe test that by manually changingconcurrent-ruby/lib/concurrent/collection/map/non_concurrent_map_backend.rb
and replacing@backend
with@__backend__
.PS: Rails doesn’t iterate the said
C::Map
s, does it?