[FIX] openerp.api.Environment: move recomputation todos into a shared…

… object

This fixes a bug which is usually triggered in module account_followup, but
does not occur deterministically.  Some recomputations of computed fields are
apparently missing.  Environment objects containing recomputations todos and
kept alive by a WeakSet, are removed by the Python garbage collector before
recomputation takes place.  We fix the bug by moving the recomputation todos in
a non-weakref'ed object.

openerp/api.py

@@ -59,6 +59,7 @@
 ]
 
 import logging
+import operator
 
 from inspect import currentframe, getargspec
 from collections import defaultdict, MutableMapping
@@ -685,7 +686,7 @@ def manage(cls):
             yield
         else:
             try:
-                cls._local.environments = WeakSet()
+                cls._local.environments = Environments()
                 yield
             finally:
                 release_local(cls._local)
@@ -708,8 +709,6 @@ def __new__(cls, cr, uid, context):
         self.prefetch = defaultdict(set)    # {model_name: set(id), ...}
         self.computed = defaultdict(set)    # {field: set(id), ...}
         self.dirty = set()                  # set(record)
-        self.todo = {}                      # {field: records, ...}
-        self.mode = env.mode if env else Mode()
         self.all = envs
         envs.add(self)
         return self
@@ -746,14 +745,14 @@ def lang(self):
 
     @contextmanager
     def _do_in_mode(self, mode):
-        if self.mode.value:
+        if self.all.mode:
             yield
         else:
             try:
-                self.mode.value = mode
+                self.all.mode = mode
                 yield
             finally:
-                self.mode.value = False
+                self.all.mode = False
                 self.dirty.clear()
 
     def do_in_draft(self):
@@ -765,7 +764,7 @@ def do_in_draft(self):
     @property
     def in_draft(self):
         """ Return whether we are in draft mode. """
-        return bool(self.mode.value)
+        return bool(self.all.mode)
 
     def do_in_onchange(self):
         """ Context-switch to 'onchange' draft mode, which is a specialized
@@ -776,7 +775,7 @@ def do_in_onchange(self):
     @property
     def in_onchange(self):
         """ Return whether we are in 'onchange' draft mode. """
-        return self.mode.value == 'onchange'
+        return self.all.mode == 'onchange'
 
     def invalidate(self, spec):
         """ Invalidate some fields for some records in the cache of all
@@ -788,7 +787,7 @@ def invalidate(self, spec):
         """
         if not spec:
             return
-        for env in list(iter(self.all)):
+        for env in list(self.all):
             c = env.cache
             for field, ids in spec:
                 if ids is None:
@@ -801,12 +800,49 @@ def invalidate(self, spec):
 
     def invalidate_all(self):
         """ Clear the cache of all environments. """
-        for env in list(iter(self.all)):
+        for env in list(self.all):
             env.cache.clear()
             env.prefetch.clear()
             env.computed.clear()
             env.dirty.clear()
 
+    def field_todo(self, field):
+        """ Check whether `field` must be recomputed, and returns a recordset
+            with all records to recompute for `field`.
+        """
+        if field in self.all.todo:
+            return reduce(operator.or_, self.all.todo[field])
+
+    def check_todo(self, field, record):
+        """ Check whether `field` must be recomputed on `record`, and if so,
+            returns the corresponding recordset to recompute.
+        """
+        for recs in self.all.todo.get(field, []):
+            if recs & record:
+                return recs
+
+    def add_todo(self, field, records):
+        """ Mark `field` to be recomputed on `records`. """
+        recs_list = self.all.todo.setdefault(field, [])
+        recs_list.append(records)
+
+    def remove_todo(self, field, records):
+        """ Mark `field` as recomputed on `records`. """
+        recs_list = self.all.todo.get(field, [])
+        if records in recs_list:
+            recs_list.remove(records)
+            if not recs_list:
+                del self.all.todo[field]
+
+    def has_todo(self):
+        """ Return whether some fields must be recomputed. """
+        return bool(self.all.todo)
+
+    def get_todo(self):
+        """ Return a pair `(field, records)` to recompute. """
+        for field, recs_list in self.all.todo.iteritems():
+            return field, recs_list[0]
+
     def check_cache(self):
         """ Check the cache consistency. """
         # make a full copy of the cache, and invalidate it
@@ -835,9 +871,20 @@ def check_cache(self):
             raise Warning('Invalid cache for fields\n' + pformat(invalids))
 
 
-class Mode(object):
-    """ A mode flag shared among environments. """
-    value = False           # False, True (draft) or 'onchange' (onchange draft)
+class Environments(object):
+    """ A common object for all environments in a request. """
+    def __init__(self):
+        self.envs = WeakSet()           # weak set of environments
+        self.todo = {}                  # recomputations {field: [records]}
+        self.mode = False               # flag for draft/onchange
+
+    def add(self, env):
+        """ Add the environment `env`. """
+        self.envs.add(env)
+
+    def __iter__(self):
+        """ Iterate over environments. """
+        return iter(self.envs)
 
 
 # keep those imports here in order to handle cyclic dependencies correctly

openerp/models.py

@@ -3149,9 +3149,9 @@ def _prefetch_field(self, field):
         if self.env.in_draft:
             # we may be doing an onchange, do not prefetch other fields
             pass
-        elif field in self.env.todo:
+        elif self.env.field_todo(field):
             # field must be recomputed, do not prefetch records to recompute
-            records -= self.env.todo[field]
+            records -= self.env.field_todo(field)
         elif self._columns[field.name]._prefetch:
             # here we can optimize: prefetch all classic and many2one fields
             fnames = set(fname
@@ -5498,42 +5498,34 @@ def _recompute_check(self, field):
         """ If `field` must be recomputed on some record in `self`, return the
             corresponding records that must be recomputed.
         """
-        for env in [self.env] + list(iter(self.env.all)):
-            if env.todo.get(field) and env.todo[field] & self:
-                return env.todo[field]
+        return self.env.check_todo(field, self)
 
     def _recompute_todo(self, field):
         """ Mark `field` to be recomputed. """
-        todo = self.env.todo
-        todo[field] = (todo.get(field) or self.browse()) | self
+        self.env.add_todo(field, self)
 
     def _recompute_done(self, field):
-        """ Mark `field` as being recomputed. """
-        todo = self.env.todo
-        if field in todo:
-            recs = todo.pop(field) - self
-            if recs:
-                todo[field] = recs
+        """ Mark `field` as recomputed. """
+        self.env.remove_todo(field, self)
 
     @api.model
     def recompute(self):
         """ Recompute stored function fields. The fields and records to
             recompute have been determined by method :meth:`modified`.
         """
-        for env in list(iter(self.env.all)):
-            while env.todo:
-                field, recs = next(env.todo.iteritems())
-                # evaluate the fields to recompute, and save them to database
-                for rec, rec1 in zip(recs, recs.with_context(recompute=False)):
-                    try:
-                        values = rec._convert_to_write({
-                            f.name: rec[f.name] for f in field.computed_fields
-                        })
-                        rec1._write(values)
-                    except MissingError:
-                        pass
-                # mark the computed fields as done
-                map(recs._recompute_done, field.computed_fields)
+        while self.env.has_todo():
+            field, recs = self.env.get_todo()
+            # evaluate the fields to recompute, and save them to database
+            for rec, rec1 in zip(recs, recs.with_context(recompute=False)):
+                try:
+                    values = rec._convert_to_write({
+                        f.name: rec[f.name] for f in field.computed_fields
+                    })
+                    rec1._write(values)
+                except MissingError:
+                    pass
+            # mark the computed fields as done
+            map(recs._recompute_done, field.computed_fields)
 
     #
     # Generic onchange method