odoo
/
odoo
2
0
Fork 0
Code Pull Requests Releases Activity

[FIX] ir.attachment: less non-transactional side-effects during deletion 

When deleting filesystem-backed attachements, the
deletion on the file-system is not transactional.
In the event of a transaction rollback, the file
deletion would not be rolled back, which is a
dangerous side-effect.

This can happen for example when several transactions
try to delete the same file(s) at the same time.
The duplicate deletions might be detected by the
database (being concurrent update errors), and rolled
back at the point of the DELETE query, to be retried.
If the files have already been deleted in the file
system it before the rollback, it leaves the system
in an inconsistent state, at least temporarily.

One case where we have seen it is when web bundles
are loaded by many web users at the same time, right
after being updated (and thus invalidated).
As they are currently cached as ir.attachment records,
this often causes a corruption of the cache.
This commit is contained in:
Olivier Dony 2014-09-03 18:39:25 +02:00
parent 46784659f9
commit a03dfefda9
1 changed files with 21 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
openerp/addons/base/ir/ir_attachment.py
View File

@ -138,9 +138,9 @@ class ir_attachment(osv.osv):
return fname
def _file_delete(self, cr, uid, fname):
count = self.search(cr, 1, [('store_fname','=',fname)], count=True)
count = self.search_count(cr, 1, [('store_fname','=',fname)])
full_path = self._full_path(cr, uid, fname)
if count <= 1 and os.path.exists(full_path):
if not count and os.path.exists(full_path):
try:
os.unlink(full_path)
except OSError:
@ -170,14 +170,18 @@ class ir_attachment(osv.osv):
location = self._storage(cr, uid, context)
file_size = len(value.decode('base64'))
attach = self.browse(cr, uid, id, context=context)
if attach.store_fname:
self._file_delete(cr, uid, attach.store_fname)
fname_to_delete = attach.store_fname
if location != 'db':
fname = self._file_write(cr, uid, value)
# SUPERUSER_ID as probably don't have write access, trigger during create
super(ir_attachment, self).write(cr, SUPERUSER_ID, [id], {'store_fname': fname, 'file_size': file_size, 'db_datas': False}, context=context)
else:
super(ir_attachment, self).write(cr, SUPERUSER_ID, [id], {'db_datas': value, 'file_size': file_size, 'store_fname': False}, context=context)
# After de-referencing the file in the database, check whether we need
# to garbage-collect it on the filesystem
if fname_to_delete:
self._file_delete(cr, uid, fname_to_delete)
return True
_name = 'ir.attachment'
@ -315,10 +319,19 @@ class ir_attachment(osv.osv):
if isinstance(ids, (int, long)):
ids = [ids]
self.check(cr, uid, ids, 'unlink', context=context)
for attach in self.browse(cr, uid, ids, context=context):
if attach.store_fname:
self._file_delete(cr, uid, attach.store_fname)
return super(ir_attachment, self).unlink(cr, uid, ids, context)
# First delete in the database, *then* in the filesystem if the
# database allowed it. Helps avoid errors when concurrent transactions
# are deleting the same file, and some of the transactions are
# rolled back by PostgreSQL (due to concurrent updates detection).
to_delete = [a.store_fname
for a in self.browse(cr, uid, ids, context=context)
if a.store_fname]
res = super(ir_attachment, self).unlink(cr, uid, ids, context)
for file_path in to_delete:
self._file_delete(cr, uid, file_path)
return res
def create(self, cr, uid, values, context=None):
self.check(cr, uid, [], mode='write', context=context, values=values)