SystemStackError during destroy with ActiveStorage 5.2

Question

SystemStackError during destroy with ActiveStorage 5.2

domcleal opened this issue 7 years ago · 14 comments

When using both acts_as_paranoid and ActiveStorage on the same model, destroying the model causes a SystemStackError as the stack overflows with lots of nested callbacks around ActiveStorage::Attachment purging.

As a minimum this error shouldn't occur, but some fuller support for soft deletion with ActiveStorage would be great, allowing for the attachment+blob to remain until really deleted.

Here's a simple test application showing the error: https://github.com/domcleal/acts_as_paranoid_with_activestorage

With this model:

class Post < ApplicationRecord
  acts_as_paranoid
  has_one_attached :file
end

Example stack trace:

    activestorage (5.2.0) app/models/active_storage/attachment.rb:28:in `purge_later'
    activestorage (5.2.0) lib/active_storage/attached/one.rb:67:in `purge_later'
    activestorage (5.2.0) lib/active_storage/attached/macros.rb:47:in `block in has_one_attached'
    activesupport (5.2.0) lib/active_support/callbacks.rb:426:in `instance_exec'
    activesupport (5.2.0) lib/active_support/callbacks.rb:426:in `block in make_lambda'
    activesupport (5.2.0) lib/active_support/callbacks.rb:261:in `block in conditional'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `block in invoke_after'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `each'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `invoke_after'
    activesupport (5.2.0) lib/active_support/callbacks.rb:133:in `run_callbacks'
    activesupport (5.2.0) lib/active_support/callbacks.rb:816:in `_run_commit_callbacks'
    activerecord (5.2.0) lib/active_record/transactions.rb:345:in `committed!'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:119:in `commit_records'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:214:in `block in commit_transaction'
    /home/dominic/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:204:in `commit_transaction'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:243:in `block in within_new_transaction'
    /home/dominic/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:227:in `within_new_transaction'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/database_statements.rb:254:in `transaction'
    activerecord (5.2.0) lib/active_record/transactions.rb:212:in `transaction'
    activerecord (5.2.0) lib/active_record/transactions.rb:383:in `with_transaction_returning_status'
    activerecord (5.2.0) lib/active_record/transactions.rb:305:in `destroy'
    activestorage (5.2.0) app/models/active_storage/attachment.rb:28:in `purge_later'
    activestorage (5.2.0) lib/active_storage/attached/one.rb:67:in `purge_later'
    activestorage (5.2.0) lib/active_storage/attached/macros.rb:47:in `block in has_one_attached'
    activesupport (5.2.0) lib/active_support/callbacks.rb:426:in `instance_exec'
    activesupport (5.2.0) lib/active_support/callbacks.rb:426:in `block in make_lambda'
    activesupport (5.2.0) lib/active_support/callbacks.rb:261:in `block in conditional'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `block in invoke_after'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `each'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `invoke_after'
    activesupport (5.2.0) lib/active_support/callbacks.rb:133:in `run_callbacks'
    activesupport (5.2.0) lib/active_support/callbacks.rb:816:in `_run_commit_callbacks'
    activerecord (5.2.0) lib/active_record/transactions.rb:345:in `committed!'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:119:in `commit_records'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:214:in `block in commit_transaction'
    /home/dominic/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:204:in `commit_transaction'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:243:in `block in within_new_transaction'
    /home/dominic/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:227:in `within_new_transaction'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/database_statements.rb:254:in `transaction'
    activerecord (5.2.0) lib/active_record/transactions.rb:212:in `transaction'
    activerecord (5.2.0) lib/active_record/transactions.rb:383:in `with_transaction_returning_status'
    activerecord (5.2.0) lib/active_record/transactions.rb:305:in `destroy'
    activestorage (5.2.0) app/models/active_storage/attachment.rb:28:in `purge_later'
    activestorage (5.2.0) lib/active_storage/attached/one.rb:67:in `purge_later'
    activestorage (5.2.0) lib/active_storage/attached/macros.rb:47:in `block in has_one_attached'
    activesupport (5.2.0) lib/active_support/callbacks.rb:426:in `instance_exec'
    activesupport (5.2.0) lib/active_support/callbacks.rb:426:in `block in make_lambda'
    activesupport (5.2.0) lib/active_support/callbacks.rb:261:in `block in conditional'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `block in invoke_after'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `each'
    activesupport (5.2.0) lib/active_support/callbacks.rb:517:in `invoke_after'
    activesupport (5.2.0) lib/active_support/callbacks.rb:133:in `run_callbacks'
    activesupport (5.2.0) lib/active_support/callbacks.rb:816:in `_run_commit_callbacks'
    activerecord (5.2.0) lib/active_record/transactions.rb:345:in `committed!'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:119:in `commit_records'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:214:in `block in commit_transaction'
    /home/dominic/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:204:in `commit_transaction'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:243:in `block in within_new_transaction'
    /home/dominic/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/monitor.rb:226:in `mon_synchronize'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:227:in `within_new_transaction'
    activerecord (5.2.0) lib/active_record/connection_adapters/abstract/database_statements.rb:254:in `transaction'
    activerecord (5.2.0) lib/active_record/transactions.rb:212:in `transaction'
    activerecord (5.2.0) lib/active_record/transactions.rb:383:in `with_transaction_returning_status'
    /home/dominic/.rvm/gems/ruby-2.5.0/bundler/gems/acts_as_paranoid-a86ee2c5350b/lib/acts_as_paranoid/core.rb:122:in `destroy!'
    test/models/post_test.rb:9:in `block in <class:PostTest>'

Answer 1 · 2018-11-09T00:15:50.000Z

What's the status of this issue? Is there any temporary fix? I'm having issues with it and unable to use acts_as_paranoid with active storage

Answer 2 · 2018-11-09T00:22:35.000Z

For anyone else dealing with this, I'm currently referencing @aymeric-ledorze fork which has a PR up. This is not ideal as I can't move this forked gem to production, but it's the best solution I have at the moment.

Answer 3 · 2019-01-05T11:54:43.000Z

I am facing the same issue. How to deal with it?

Answer 4 · 2019-01-05T12:24:02.000Z

Quick fix. Replace
has_one_attached :file
with
has_one_attached :file, dependent: :purge_now

I still have to figure out how to deal with :purge_later

Answer 5 · 2019-09-03T08:10:27.000Z

Any resolution on how to soft delete without performing the purge callback until it is fully destroyed?

Answer 6 · 2020-04-03T19:20:58.000Z

Any updates on this?

Answer 7 · 2020-05-21T17:44:18.000Z

Yeah, would love to use this gem, but don't want to purge the documents

Answer 8 · 2020-06-17T18:22:36.000Z

Any updates on this?

Answer 9 · 2021-04-14T18:37:32.000Z

So I've got a possible solution that seems to work. I would appreciate any feedback as it doesn't feel particularly solid.

In my app Employees have files and I want to be able to archive the employee, then get their files back when restoring the Employee record.

#/models/employee.rb
class Employee < ApplicationRecord
acts_as_paranoid  

def destroy
    @employee = Employee.find(params[:id])
    
    #create a temporary employee object to attach all files to.
    @temp_employee = Employee.new(name:"temp", position: "temp")

    #attach each of the original employee's files to the temp employee
    @employee.files.each do |f|
      @temp_employee.files.attach(f.blob)
    end

    #soft delete the employee
    @employee.destroy
    
    #re-attach all the files back 
    @temp_employee.files.each do |n|
      @employee.files.attach(n.blob)
    end
    
    @employee.save! 
   #employee is soft deleted, and the files are attached to the record
end
end

UPDATE: as I extended this it got increasingly complicated and I ended up switching to discard instead as it seems more suited to this use case.

Answer 10 · 2023-10-06T22:48:46.000Z

Hello folks, is this issue got resolved or not yet ?

Answer 11 · 2023-10-08T22:06:07.000Z

@navidemad as far as I know this is still a problem.

Answer 12 · 2023-10-10T15:39:08.000Z

I have a test with ActiveStorage in my PR #108 and it seems that the crash is not there anymore. It looks like it got solved by itself with Rails 6.0. Maybe there is another way to trigger this problem but my test does not cover it.
However, the attachments and the blobs are still lost so this is not a soft deletion yet.

Answer 13 · 2023-12-04T14:24:33.000Z

So I've got a possible solution that seems to work. I would appreciate any feedback as it doesn't feel particularly solid.

In my app Employees have files and I want to be able to archive the employee, then get their files back when restoring the Employee record.

#/models/employee.rb
class Employee < ApplicationRecord
acts_as_paranoid  

def destroy
    @employee = Employee.find(params[:id])
    
    #create a temporary employee object to attach all files to.
    @temp_employee = Employee.new(name:"temp", position: "temp")

    #attach each of the original employee's files to the temp employee
    @employee.files.each do |f|
      @temp_employee.files.attach(f.blob)
    end

    #soft delete the employee
    @employee.destroy
    
    #re-attach all the files back 
    @temp_employee.files.each do |n|
      @employee.files.attach(n.blob)
    end
    
    @employee.save! 
   #employee is soft deleted, and the files are attached to the record
end
end

UPDATE: as I extended this it got increasingly complicated and I ended up switching to discard instead as it seems more suited to this use case.

@jonsinclair1 How did you manage to save employee? I get an error when I try to save the soft-deleted record.

Answer 14 · 2024-01-16T18:38:19.000Z

It's a workaround, but you could consider moving the file upload to a separate model that is not soft-deleted. I haven't tested this, though.

class Employee
  has_one :file_upload   # don't use a `dependent` option 

  has_many :tasks, dependent: :destroy
end

class FileUpload
  has_one_attached :file
  belongs_to :employee
end

employee.destroy #=> soft-deletes tasks, keeps file_upload around