From 13beccbb06089886feea1a78d169825345a46c68 Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Mon, 21 Dec 2020 10:07:13 +0800 Subject: [PATCH] introduces an additional check before removing a deduplicated file - this handles a race condition that occurs when multiple identical files are uploaded in parallel application processes. In that case it previously could have happened that, while deduplication of one attachment took place, deduplication of another attachment would change it to point to the first attachment's diskfile which then would be removed. --- app/models/attachment.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/models/attachment.rb b/app/models/attachment.rb index ad76c06df..b8d358c04 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -479,16 +479,15 @@ class Attachment < ActiveRecord::Base private def reuse_existing_file_if_possible - original_diskfile = nil + original_diskfile = diskfile + original_filename = disk_filename reused = with_lock do if existing = Attachment .where(digest: self.digest, filesize: self.filesize) - .where('id <> ? and disk_filename <> ?', - self.id, self.disk_filename) + .where.not(disk_filename: original_filename) .order(:id) .last existing.with_lock do - original_diskfile = self.diskfile existing_diskfile = existing.diskfile if File.readable?(original_diskfile) && File.readable?(existing_diskfile) && @@ -499,7 +498,7 @@ class Attachment < ActiveRecord::Base end end end - if reused + if reused && Attachment.where(disk_filename: original_filename).none? File.delete(original_diskfile) end rescue ActiveRecord::StatementInvalid, ActiveRecord::RecordNotFound -- 2.20.1