Merge lp:~openerp-dev/openobject-server/trunk-registry-lock-vmt into lp:openobject-server

[Vo Minh Thu]

Unmerged revisions

3831. By Vo Minh Thu on 2011-11-28

[IMP] registries: with lock, instead of acquire/release.

3830. By Vo Minh Thu on 2011-11-28

[MERGE] merged trunk.

3829. By Vo Minh Thu on 2011-11-28

[IMP] registries locking behavior:
RegistryManager.get() must always be able to return a currently
being initialized registry. This is true for the thread doing
the initialization. For other threads, get() should return only
completely initialized registries. (This is more consistent as
the first call to get() has thus the same behavior for any
calling thread.)
We also make sure the the cron master thread never ends up
calling new() by making possible to call get() in a non-
blocking way.

=== modified file 'openerp/cron.py'
--- openerp/cron.py	2011-09-28 23:54:09 +0000
+++ openerp/cron.py	2011-11-28 13:15:27 +0000
@@ -177,8 +177,11 @@
             if canceled:
                 continue
             del _wakeup_by_db[db_name]
-            registry = openerp.pooler.get_pool(db_name)
-            if not registry._init:
+            # The master cron thread never initilizes itself a new registry,
+            # so we use blocking=False and don't need to test registry._init.
+            registry = openerp.modules.registry.RegistryManager.get(
+                db_name, blocking=False)
+            if registry:
                 _logger.debug("Database '%s' wake-up! Firing multi-threaded cron job processing", db_name)
                 registry['ir.cron']._run_jobs_multithread()
     amount = MAX_SLEEP

=== modified file 'openerp/modules/registry.py'
--- openerp/modules/registry.py	2011-09-30 15:00:26 +0000
+++ openerp/modules/registry.py	2011-11-28 13:15:27 +0000
@@ -42,6 +42,7 @@
     """
 
     def __init__(self, db_name):
+        self.lock = threading.RLock()
         self.models = {} # model name/model instance mapping
         self._sql_error = {}
         self._store_function = {}
@@ -121,6 +122,17 @@
         The manager is responsible for creation and deletion of model
         registries (essentially database connection/model registry pairs).
 
+        As the OpenERP server is multi-threaded and implements a lazy-loading
+        of registries, the interaction between get() and new() has to ensure a
+        few points:
+          - One RegistryManager.new() execution at a time: all the module
+            loading code is not thread-safe so it is protected from here.
+          - It must be possible to get() an already loaded registry while
+            another registre is being loaded.
+          - get() must return a completely initialized registry (or None if
+            it must not block). The thread doing the initialization can get
+            a partially initialized registry. It can then check its _init
+            attribute.
     """
     # Mapping between db name and model registry.
     # Accessed through the methods below.
@@ -129,13 +141,21 @@
 
     @classmethod
     def get(cls, db_name, force_demo=False, status=None, update_module=False,
-            pooljobs=True):
+            pooljobs=True, blocking=True):
         """ Return a registry for a given database name."""
         try:
-            return cls.registries[db_name]
-        except KeyError:
-            return cls.new(db_name, force_demo, status,
-                           update_module, pooljobs)
+            registry = cls.registries[db_name]
+            if registry.lock.acquire(blocking):
+                registry.lock.release()
+            else:
+                registry = None
+        except KeyError, e:
+            if blocking:
+                registry = cls.new(db_name, force_demo, status,
+                               update_module, pooljobs)
+            else:
+                registry = None
+        return registry
 
     @classmethod
     def new(cls, db_name, force_demo=False, status=None,
@@ -146,29 +166,31 @@
 
         """
         import openerp.modules
+
         with cls.registries_lock:
             registry = Registry(db_name)
-
-            # Initializing a registry will call general code which will in turn
-            # call registries.get (this object) to obtain the registry being
-            # initialized. Make it available in the registries dictionary then
-            # remove it if an exception is raised.
-            cls.delete(db_name)
-            cls.registries[db_name] = registry
-            try:
-                # This should be a method on Registry
-                openerp.modules.load_modules(registry.db, force_demo, status, update_module)
-            except Exception:
-                del cls.registries[db_name]
-                raise
-
-            cr = registry.db.cursor()
-            try:
-                registry.do_parent_store(cr)
-                registry.get('ir.actions.report.xml').register_all(cr)
-                cr.commit()
-            finally:
-                cr.close()
+            with registry.lock:
+
+                # Initializing a registry will call general code which will in turn
+                # call registries.get (this object) to obtain the registry being
+                # initialized. Make it available in the registries dictionary then
+                # remove it if an exception is raised.
+                cls.delete(db_name)
+                cls.registries[db_name] = registry
+                try:
+                    # This should be a method on Registry
+                    openerp.modules.load_modules(registry.db, force_demo, status, update_module)
+                except Exception:
+                    del cls.registries[db_name]
+                    raise
+
+                cr = registry.db.cursor()
+                try:
+                    registry.do_parent_store(cr)
+                    registry.get('ir.actions.report.xml').register_all(cr)
+                    cr.commit()
+                finally:
+                    cr.close()
 
         if pooljobs:
             registry.schedule_cron_jobs()