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

SystemStackError during destroy with ActiveStorage 5.2 #103

Open
domcleal opened this issue May 4, 2018 · 14 comments
Open

SystemStackError during destroy with ActiveStorage 5.2 #103

domcleal opened this issue May 4, 2018 · 14 comments
Assignees

Comments

@domcleal
Copy link

domcleal commented May 4, 2018

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>'
@KidA001
Copy link

KidA001 commented Nov 9, 2018

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

@KidA001
Copy link

KidA001 commented Nov 9, 2018

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.

@amitpatelx
Copy link

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

@amitpatelx
Copy link

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

@nitsujri
Copy link

nitsujri commented Sep 3, 2019

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

aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 28, 2019
@LucasKuhn
Copy link

Any updates on this?

@adenta
Copy link

adenta commented May 21, 2020

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

@mvz mvz self-assigned this May 22, 2020
@17100262
Copy link

17100262 commented Jun 17, 2020

Any updates on this?

aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Nov 20, 2020
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Nov 20, 2020
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Nov 23, 2020
@jonsinclair1
Copy link

jonsinclair1 commented Apr 14, 2021

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.

aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 27, 2021
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 27, 2021
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 27, 2021
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 27, 2021
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 27, 2021
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Apr 13, 2022
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Apr 13, 2022
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Jul 4, 2023
@navidemad
Copy link

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

@mvz
Copy link
Contributor

mvz commented Oct 8, 2023

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

aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 10, 2023
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 10, 2023
aymeric-ledorze added a commit to aymeric-ledorze/acts_as_paranoid that referenced this issue Oct 10, 2023
@aymeric-ledorze
Copy link
Contributor

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.

@Olgagr
Copy link

Olgagr commented Dec 4, 2023

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.

@ryan-secondsight
Copy link

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

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

No branches or pull requests