Fix infinite loop issue in identity comparison#1
Fix infinite loop issue in identity comparison#1krainboltgreene wants to merge 16 commits intogoatapp:masterfrom
Conversation
|
|
||
| def remove_unpersisted_records! | ||
| @records.each do |_, set| | ||
| set.instance_variable_get(:@hash).rehash |
There was a problem hiding this comment.
I think this may rely on ActiveRecord's sync_with_transaction_state being invoked upstream. From the AR source, that method is lazily executed, and if it's not run, I think rehash will have no effect.
For that reason, I think it's clearer to invoke explicitly here (as suggested in godaddy#14), whether it's via the private method itself or some public API.
There was a problem hiding this comment.
Then I'm surprised this works? Hmm.
lib/activerecord/delay_touching.rb
Outdated
| # Apply the touches that were delayed. | ||
| def self.apply | ||
| begin | ||
| # If we don't do this then an infinite loop is possible due to how Set#subtract and ActiveRecord::Core#== work |
There was a problem hiding this comment.
Not to nit pick, but I think Set#subtract and ActiveRecord::Core#== work as expected (even together), it's just that the object unintuitively changes in memory after it was used as a hash key. The fact that Hash#rehash already exists means that it's not that uncommon an issue; the problem here is that effects of the id-unsetting logic in ActiveRecord are not immediately obvious.
Would be in favor of clarifying this comment (or clarifying it and moving it to the definition of remove_unpersisted_records).
2 participants
See: godaddy#14 for more details.
Solution:
{nil => Set.new}.present? == true)