Skip to content
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

find_by_id causes stack overflow in after_find callback #26320

Closed
johnnaegle opened this issue Aug 29, 2016 · 0 comments
Closed

find_by_id causes stack overflow in after_find callback #26320

johnnaegle opened this issue Aug 29, 2016 · 0 comments

Comments

@johnnaegle
Copy link

Steps to reproduce

An executable test case is below. It demonstrates that find_by_id behaves differently than find(id), where(:id => id).first, and find_by(:id => id). find_by_id is raising a SystemStackError when including has_many relationships and that relationship has an after_find callback that access the parent object.

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  gem "rails", github: "rails/rails"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :foods, force: true do |t|
  end

  create_table :measurements, force: true do |t|
    t.integer :food_id
    t.string :name
    t.boolean :default
  end
end

class Food < ActiveRecord::Base
  has_many :measurements, :inverse_of => :food
end

class Measurement < ActiveRecord::Base
  belongs_to :food, :inverse_of => :measurements

  after_find do
    puts "Triggered after find"
    self.name = 'serving' if name.blank? && default? && food.present?
  end
end

class BugTest < Minitest::Test
  def setup
    @food = Food.create!
    @measurement = @food.measurements.create!(:default => true)
  end

  def test_find_does_not_stack_overflow
    Food.includes(:measurements).find(@food.id)
  end

  def test_where_does_not_stack_overflow
    Food.includes(:measurements).where(:id => @food.id).first
  end

  def test_find_by_does_not_stack_overflow
    Food.includes(:measurements).find_by(:id => @food.id)
  end

  def test_find_by_id_does_not_stack_overflow
    Food.find_by_id(@food.id)
  end

  def test_find_by_id_with_joins_does_not_stack_overflow
    Food.joins(:measurements).find_by_id(@food.id)
  end

  def test_find_by_id_with_includes_does_not_stack_overflow
    begin
      Food.includes(:measurements).find_by_id(@food.id)
    rescue SystemStackError => e
      raise "find_by_id with includes produced a stack overflow"
    end
  end

  def test_find_by_id_with_joins_and_includes_does_not_stack_overflow
    begin
      Food.joins(:measurements).includes(:measurements).find_by_id(@food.id)
    rescue SystemStackError => e
      raise "find_by_id with includes and joins produced a stack overflow"
    end
  end
end

This test fails with:

Finished in 1.063656s, 6.5811 runs/s, 0.0000 assertions/s.

  1) Error:
BugTest#test_find_by_id_with_joins_and_includes_does_not_stack_overflow:
RuntimeError: find_by_id with includes and joins produced a stack overflow
    wtfrails.rb:84:in `rescue in test_find_by_id_with_joins_and_includes_does_not_stack_overflow'
    wtfrails.rb:81:in `test_find_by_id_with_joins_and_includes_does_not_stack_overflow'


  2) Error:
BugTest#test_find_by_id_with_includes_does_not_stack_overflow:
RuntimeError: find_by_id with includes produced a stack overflow
    wtfrails.rb:76:in `rescue in test_find_by_id_with_includes_does_not_stack_overflow'
    wtfrails.rb:73:in `test_find_by_id_with_includes_does_not_stack_overflow'

7 runs, 0 assertions, 0 failures, 2 errors, 0 skips

Expected behavior

find_by_id, where(:id => id).first, find_by(:id => id) and find should all have the same callback behavior. They should not raise SystemStackError

Actual behavior

SystemStackError is raised.

System configuration

Rails version:

  • activerecord 5.1.0.alpha
  • activerecord 4.2.6

Ruby version:

  • ruby 2.3.1p112
@sgrif sgrif closed this as completed in caa178c Aug 31, 2016
sgrif added a commit to sgrif/rails that referenced this issue Aug 31, 2016
If a parent association was accessed in an `after_find` or
`after_initialize` callback, it would always end up loading the
association, and then immediately overwriting the association we just
loaded. If this occurred in a way that the parent's `current_scope` was
set to eager load the child, this would result in an infinite loop and
eventually overflow the stack.

For records that are created with `.new`, we have a mechanism to
perform an action before the callbacks are run. I've introduced the same
code path for records created with `instantiate`, and updated all code
which sets inverse instances on newly loaded associations to use this
block instead.

Fixes rails#26320.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant