Rails 6.1 (can't modify frozen attributes) error when destroying child record with touch: true #40943

javinto · 2020-12-26T22:11:18Z

Steps to reproduce

I found this one using AweSomeNestedSet gem, but can reproduce it in a clean model situation. I have created a MiniTest but unfortunately the error does not trigger there. See the Console steps further on where I can reproduce it.
CORRECTION: as 'doits' found out, I added the config.active_record.has_many_inversing=true option which triggers te MiniTest to generate the expected errors.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "6.1.0"
  gem "activesupport", "6.1.0"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"
require 'active_support/testing/assertions'

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

# This line is important for triggering the failure
ActiveRecord::Base.has_many_inversing = true

ActiveRecord::Schema.define do  
  create_table :pages, force: true do |t|
      t.string :name
      t.bigint :parent_id, foreign_key: true

      t.timestamps
  end

  create_table :page_comments do |t|
    t.string :name
    t.references :page, null: false, foreign_key: true
  end

end

class Page < ActiveRecord::Base
  belongs_to :parent, class_name: 'Page', foreign_key: 'parent_id', optional: true, touch: true, inverse_of: :children

  has_many :children, class_name: 'Page', foreign_key: 'parent_id', inverse_of: :parent
  has_many :comments, class_name: 'PageComment', inverse_of: :page
end

class PageComment < ActiveRecord::Base
  belongs_to :page, touch: true, inverse_of: :comments
end

class BugTest < Minitest::Test
  include ActiveSupport::Testing::Assertions

  def test_association_stuff
    root= Page.create!(name: 'parent')
    child= root.children.create!(name: 'child')
    child.reload
    
    assert_difference 'Page.count', -1 do
      child.destroy
    end

  end

  def test_association_stuff2
    root= Page.create!(name: 'parent')
    child= root.comments.create!(name: 'child')
    child.reload
    
    assert_difference 'PageComment.count', -1 do
      child.destroy
    end

  end

end

It's important to reload the child. Without it, it does not occur. Both testcases should fail. The console steps to reproduce:

2.7.2 :003 > root=Page.create(name: 'test')
  TRANSACTION (0.2ms)  BEGIN
  Page Create (0.2ms)  INSERT INTO `pages` (`name`, `created_at`, `updated_at`) VALUES ('test', '2020-12-26 21:55:18.040069', '2020-12-26 21:55:18.040069')
  TRANSACTION (0.6ms)  COMMIT
 => #<Page id: 9, name: "test", parent_id: nil, created_at: "2020-12-26 22:55:18.040069000 +0100", updated_at: "2020-12-26 22:55:18.040069000 +0100"> 
2.7.2 :004 > child= root.children.create(name: 'child')
  TRANSACTION (0.2ms)  BEGIN
  Page Create (0.2ms)  INSERT INTO `pages` (`name`, `parent_id`, `created_at`, `updated_at`) VALUES ('child', 9, '2020-12-26 21:55:32.823464', '2020-12-26 21:55:32.823464')
  Page Update (0.2ms)  UPDATE `pages` SET `pages`.`updated_at` = '2020-12-26 21:55:32.824879' WHERE `pages`.`id` = 9
  TRANSACTION (0.3ms)  COMMIT
 => #<Page id: 10, name: "child", parent_id: 9, created_at: "2020-12-26 22:55:32.823464000 +0100", updated_at: "2020-12-26 22:55:32.823464000 +0100"> 
2.7.2 :005 > child.reload
  Page Load (0.3ms)  SELECT `pages`.* FROM `pages` WHERE `pages`.`id` = 10 LIMIT 1
 => #<Page id: 10, name: "child", parent_id: 9, created_at: "2020-12-26 22:55:32.823464000 +0100", updated_at: "2020-12-26 22:55:32.823464000 +0100"> 
2.7.2 :006 > child.destroy
  TRANSACTION (0.2ms)  BEGIN
  Page Destroy (0.2ms)  DELETE FROM `pages` WHERE `pages`.`id` = 10
  Page Load (0.2ms)  SELECT `pages`.* FROM `pages` WHERE `pages`.`id` = 9 LIMIT 1
  TRANSACTION (0.7ms)  ROLLBACK
Traceback (most recent call last):
        1: from (irb):6
FrozenError (can't modify frozen attributes)
2.7.2 :007 >

Removing the inverse_of option with the belongs_to associations solves the problem.

I have no idea why the MiniTest is not able to reproduce it.

Expected behavior

I expect the child record to be destroyed and the parent to be touched.

Actual behavior

The child record is not destroyed but triggers a FrozenError (can't modify frozen attributes)

System configuration

Rails 6.1.0:

Ruby 2.7.2:

The text was updated successfully, but these errors were encountered:

doits · 2020-12-28T15:36:21Z

I have the same problem without awesome_nested_set and can at least provide the following backtrace for 6-1-stable:

 "/rails-4bbfc87dfd37/activerecord/lib/active_record/attribute_methods/write.rb:34:in `write_attribute'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/attribute_methods.rb:345:in `[]='",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/belongs_to_association.rb:112:in `replace_keys'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/belongs_to_association.rb:32:in `inversed_from'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/association.rb:114:in `set_inverse_instance'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/collection_association.rb:456:in `replace_on_target'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/collection_association.rb:285:in `add_to_target'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/collection_association.rb:295:in `target='",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/association.rb:134:in `inversed_from'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/association.rb:114:in `set_inverse_instance'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/association.rb:231:in `block in find_target'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/core.rb:514:in `init_with_attributes'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/persistence.rb:403:in `instantiate_instance_of'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/querying.rb:66:in `block (2 levels) in find_by_sql'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/result.rb:62:in `block in each'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/result.rb:62:in `each'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/result.rb:62:in `each'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/querying.rb:66:in `map'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/querying.rb:66:in `block in find_by_sql'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/notifications/instrumenter.rb:24:in `instrument'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/querying.rb:61:in `find_by_sql'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/statement_cache.rb:150:in `execute'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/association.rb:231:in `find_target'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/singular_association.rb:39:in `find_target'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/association.rb:174:in `load_target'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/association.rb:67:in `reload'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/singular_association.rb:9:in `reader'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/builder/association.rb:102:in `event'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/builder/belongs_to.rb:68:in `public_send'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/builder/belongs_to.rb:68:in `touch_record'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/associations/builder/belongs_to.rb:84:in `block (2 levels) in add_touch_callbacks'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/callbacks.rb:427:in `instance_exec'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/callbacks.rb:427:in `block in make_lambda'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/callbacks.rb:235:in `block in halting_and_conditional'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/callbacks.rb:516:in `block in invoke_after'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/callbacks.rb:516:in `each'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/callbacks.rb:516:in `invoke_after'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/callbacks.rb:107:in `run_callbacks'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/callbacks.rb:824:in `_run_destroy_callbacks'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/callbacks.rb:439:in `destroy'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/transactions.rb:292:in `block in destroy'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/transactions.rb:352:in `block in with_transaction_returning_status'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `block in transaction'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:310:in `block in within_new_transaction'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'",
 "/rails-4bbfc87dfd37/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:308:in `within_new_transaction'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `transaction'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/transactions.rb:348:in `with_transaction_returning_status'",
 "/rails-4bbfc87dfd37/activerecord/lib/active_record/transactions.rb:292:in `destroy'",

Couldn't reproduce it in a clean app for now either.

doits · 2020-12-28T16:06:18Z

And I also noticed it only happens if the association is not preloaded, so for example:

class Event < ApplicationRecord
  has_many :applications, class_name: 'EventApplication', inverse_of: :event, dependent: :destroy
end

class EventApplication < ApplicationRecord
  belongs_to :event, inverse_of: :applications, touch: true
end

ea = EventApplication.last
ea.destroy
# => FrozenError

# but this works:
ea = EventApplication.last
ea.event # load event here!
ea.destroy
# => no error

Looks like setting/loading the association while destroying the record is a problem.

Still can't find out why it happens in my (complex) app but not in a clean test app.

doits · 2020-12-28T17:15:21Z

Woohoo, finally got a reproducible test case. The culprit lies within ActiveRecord::Base.has_many_inversing = true in combination with using id: :uuid in my case.

To reproduce it with postgresql and uuid columns:

Create a postgresql database bug_report
Run the bug template

To reproduce it with @javinto's original report:

Run this gist (bug report by @javinto with added ActiveRecord::Base.has_many_inversing = true)

Work around if you do not rely on it: Set ActiveRecord::Base.has_many_inversing = false.

javinto · 2020-12-28T19:39:26Z

Thank you Markus for figgering out. I just edited my Bugtest to be able to reproduce it.

It seems to be an inverse_of problem

rafaelfranca · 2020-12-28T22:24:30Z

Given you already can reproduce in a script, can you try to bisect the commit that broke this case and possibly open a PR fixing?

doits · 2020-12-28T22:35:51Z

Sure, will give it a shot tomorrow.

doits · 2020-12-29T10:13:53Z

The offending commit is d45c9ad – so it was broken from the beginning when has_many inversing was added. Will see later if I can find a fix for it.

rails/rails#40943

doits · 2020-12-29T17:42:20Z

I proposed a fix in #40969 ... was a lot of digging through Rails' code for such a simple fix :-)

doits · 2020-12-29T17:46:56Z

For the record here is a simpler bug template that triggers the error – only inverse_of and touch are needed in combination with has_many_inversing = true.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "rails", github: "rails/rails", branch: '6-1-stable'
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"
require 'active_support/testing/assertions'

ActiveRecord::Base.has_many_inversing = true

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

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

  create_table :event_applications, force: true do |t|
    t.references :event, null: false
  end
end

class Event < ActiveRecord::Base
  has_many :event_applications
end

class EventApplication < ActiveRecord::Base
  belongs_to :event, inverse_of: :event_applications, touch: true
end

class BugTest < Minitest::Test
  include ActiveSupport::Testing::Assertions

  def test_association_stuff
    event = Event.create!
    event.event_applications.create!

    assert_difference 'EventApplication.count', -1 do
      EventApplication.last.destroy
    end
  end
end

rails/rails#40943

…ith inverse `has_many_inversing` adds records to a has_many association. It does so with destroyed records, too. So if a child was destroyed with a `touch: true` association on the parent *and* the parent was not loaded, it tried to load the parent to touch it. While loading the parent it added the child record to the parent's has_many association. The logic doing this always set the child's parent id – even if it was correct/the same already. But since the child is destroyed, it resulted in a `FrozenError`. This commit prevents doing the unnecessary setting of the identical id and therefore fixes this error. Fixes rails#40943 by not doing an unneeded attribute set.

…ith inverse `has_many_inversing` adds records to a has_many association. It does so with destroyed records, too. So if a child was destroyed with a `touch: true` association on the parent *and* the parent was not loaded, it tried to load the parent to touch it. While loading the parent it added the child record to the parent's has_many association. The logic doing this always set the child's parent id – even if it was correct/the same already. But since the child is destroyed, it resulted in a `FrozenError`. This commit prevents doing the unnecessary setting of the identical id and therefore fixes this error. Fixes #40943 by not doing an unneeded attribute set. Closes #40969. [Markus Doits + Rafael Mendonça França]

javinto mentioned this issue Dec 26, 2020

Rails 6.1 destroy problem with touch: true causes FrozenError collectiveidea/awesome_nested_set#438

Closed

rafaelfranca added the activerecord label Dec 28, 2020

rafaelfranca added this to the 6.1.1 milestone Dec 28, 2020

damianlegawiec added a commit to spark-solutions/spree that referenced this issue Dec 29, 2020

Fixed can't modify frozen attributes

b164131

rails/rails#40943

damianlegawiec added a commit to spark-solutions/spree that referenced this issue Dec 29, 2020

Fixed can't modify frozen attributes

358b85e

rails/rails#40943

damianlegawiec added a commit to spark-solutions/spree that referenced this issue Dec 29, 2020

Fixed can't modify frozen attributes

cf3cc11

rails/rails#40943

doits mentioned this issue Dec 29, 2020

Fix has_many_inversing with touch: true on a belongs_to association with inverse #40969

Closed

damianlegawiec added a commit to spark-solutions/spree that referenced this issue Dec 29, 2020

Fixed can't modify frozen attributes

ad7c0a4

rails/rails#40943

damianlegawiec added a commit to spark-solutions/spree that referenced this issue Dec 29, 2020

Fixed can't modify frozen attributes

9d3d3ef

rails/rails#40943

damianlegawiec added a commit to spark-solutions/spree that referenced this issue Dec 29, 2020

Fixed can't modify frozen attributes

29fbe09

rails/rails#40943

rafaelfranca closed this as completed in 2dd8187 Jan 7, 2021

Aelphaeis mentioned this issue Oct 25, 2021

Support for Rails 6.1.4.1 and onward (Until 7) + Supporting CI Changes. clio/polymorphic_integer_type#47

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rails 6.1 (can't modify frozen attributes) error when destroying child record with touch: true #40943

Rails 6.1 (can't modify frozen attributes) error when destroying child record with touch: true #40943

javinto commented Dec 26, 2020 •

edited

Loading

doits commented Dec 28, 2020 •

edited

Loading

doits commented Dec 28, 2020 •

edited

Loading

doits commented Dec 28, 2020 •

edited

Loading

javinto commented Dec 28, 2020

rafaelfranca commented Dec 28, 2020

doits commented Dec 28, 2020

doits commented Dec 29, 2020

doits commented Dec 29, 2020

doits commented Dec 29, 2020 •

edited

Loading

Rails 6.1 (can't modify frozen attributes) error when destroying child record with touch: true #40943

Rails 6.1 (can't modify frozen attributes) error when destroying child record with touch: true #40943

Comments

javinto commented Dec 26, 2020 • edited Loading

Steps to reproduce

Expected behavior

Actual behavior

System configuration

doits commented Dec 28, 2020 • edited Loading

doits commented Dec 28, 2020 • edited Loading

doits commented Dec 28, 2020 • edited Loading

javinto commented Dec 28, 2020

rafaelfranca commented Dec 28, 2020

doits commented Dec 28, 2020

doits commented Dec 29, 2020

doits commented Dec 29, 2020

doits commented Dec 29, 2020 • edited Loading

javinto commented Dec 26, 2020 •

edited

Loading

doits commented Dec 28, 2020 •

edited

Loading

doits commented Dec 28, 2020 •

edited

Loading

doits commented Dec 28, 2020 •

edited

Loading

doits commented Dec 29, 2020 •

edited

Loading