From fde7727dd8fc8697f26c8025849fa27410739028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Voet=20=28ryv=29?= Date: Fri, 21 Apr 2023 13:31:05 +0000 Subject: [PATCH] [FIX] core: fix recursion error from `unlink` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a model X, where there is a field related x_related (related= 'y_id.y_translate') towards a translate field y_translate on Model Y. When you unlink at least 1001 records of X (r1, r2, ... , r1000, r1001) (cr.MAX_IN + 1), you get a traceback (`RecursionError: maximum recursion depth exceeded in comparison`). The stack looks like: File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 3609, in unlink self.env.flush_all() File "/home/odoo/Documents/dev/odoo/odoo/api.py", line 732, in flush_all self._recompute_all() File "/home/odoo/Documents/dev/odoo/odoo/api.py", line 728, in _recompute_all self[field.model_name]._recompute_field(field) File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 6179, in _recompute_field field.recompute(records) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 1348, in recompute self.compute_value(record) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 1368, in compute_value records._compute_field_value(self) File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 4209, in _compute_field_value fields.determine(field.compute, self) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 100, in determine return needle(records, *args) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 689, in _compute_related values = [first(value[name]) for value in values] File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 689, in values = [first(value[name]) for value in values] File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 5874, in __getitem__ return self._fields[key].__get__(self, type(self)) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 2771, in __get__ return super().__get__(records, owner) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 1186, in __get__ recs._fetch_field(self) File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 3162, in _fetch_field self._read(fnames) File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 3214, in _read self.flush_recordset(translated_field_names) ... -> Extra info: translated_field_names = ['x_related'] `self = X(r1001, r1, r2, ..., r999)` ... File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 5585, in flush_recordset self._recompute_recordset(fnames) File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 6162, in _recompute_recordset self._recompute_field(field, self._ids) ... -> Long recursion starts here but first records to be recomputed will be r1, then r2, then r3, ... But the maximum recursion depth error will be triggered earlier at the end of the recursion, because the stack limit in Python is 1000 by default. ... File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 6179, in _recompute_field field.recompute(records) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 1348, in recompute self.compute_value(record) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 1368, in compute_value records._compute_field_value(self) File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 4209, in _compute_field_value fields.determine(field.compute, self) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 100, in determine return needle(records, *args) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 689, in _compute_related values = [first(value[name]) for value in values] File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 689, in values = [first(value[name]) for value in values] File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 5874, in __getitem__ return self._fields[key].__get__(self, type(self)) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 2771, in __get__ return super().__get__(records, owner) File "/home/odoo/Documents/dev/odoo/odoo/fields.py", line 1186, in __get__ recs._fetch_field(self) File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 3162, in _fetch_field self._read(fnames) File "/home/odoo/Documents/dev/odoo/odoo/models.py", line 3214, in _read self.flush_recordset(translated_field_names) ... ------ Extra info: translated_field_names = ['x_related'] `self = X(r1, r2, ... , r999)` ... Since 9c3b9a4926171f88a688552320396cc30b208e39, the `x_related` is flagged to be recomputed at the end of the delete loop of `unlink`, but it shouldn't be, because records are deleted now. The problem is in these lines: ```python with self.env.protecting(self._fields.values(), records): self.modified(self._fields, before=True) ``` The `records` are only a part of `self` (batch of 1000), so the `protecting` call only protects the current batch and not `self`. Then, the second batch (here with only one record), will flag to recompute `x_related` of the first batch records. Then, later on, the `flush_all` will generate the recursion error trying to resolve these `to_recompute`. To fix it, only move the modified call (+ protecting) before the batch loop and executes it on `self`. This issue shouldn't exist in master, because having a related translate field triggers a warning (`Translated stored related field () will not be computed correctly in all languages`). Also `https://github.com/odoo/odoo/pull/100472` fixes the issue in master, but it generates one SQL request by record, which isn't great. Then in master, we should forward this commit (but test will be remove because it generates the warning message). closes odoo/odoo#119471 X-original-commit: 78450cae5900e267de439e4988b314aa644e76bd Signed-off-by: Raphael Collet Signed-off-by: Rémy Voet --- odoo/models.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/odoo/models.py b/odoo/models.py index a11463dbda9..8150c6b678f 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3797,6 +3797,11 @@ class BaseModel(metaclass=MetaModel): ir_model_data_unlink = Data ir_attachment_unlink = Attachment + # mark fields that depend on 'self' to recompute them after 'self' has + # been deleted (like updating a sum of lines after deleting one line) + with self.env.protecting(self._fields.values(), self): + self.modified(self._fields, before=True) + for sub_ids in cr.split_for_in_conditions(self.ids): records = self.browse(sub_ids) @@ -3811,11 +3816,6 @@ class BaseModel(metaclass=MetaModel): # Delete the records' properties. ir_property_unlink |= Property.search([('res_id', 'in', refs)]) - # mark fields that depend on 'self' to recompute them after 'self' has - # been deleted (like updating a sum of lines after deleting one line) - with self.env.protecting(self._fields.values(), records): - records.modified(self._fields, before=True) - query = f'DELETE FROM "{self._table}" WHERE id IN %s' cr.execute(query, (sub_ids,))