Mutex on _CONFIGURE_MUTEX in automap.prepare()

Fixed a race condition which could occur if automap
:meth:`.AutomapBase.prepare` were used within a multi-threaded context
against other threads which  may call :func:`.configure_mappers` as a
result of use of other mappers.  The unfinished mapping work of automap
is particularly sensitive to being pulled in by a
:func:`.configure_mappers` step leading to errors.

Change-Id: I6d36df6639bf5cb8f137187dff68f386f5e84f88
Fixes: #4266

diff --git a/doc/build/changelog/unreleased_12/4266.rst b/doc/build/changelog/unreleased_12/4266.rst
new file mode 100644 (file)
index 0000000..923c7c8
--- /dev/null
+++ b/doc/build/changelog/unreleased_12/4266.rst
@@ -0,0 +1,11 @@
+.. change::
+       :tags: bug, ext
+       :tickets: 4266
+       :versions: 1.3.0b1
+
+       Fixed a race condition which could occur if automap
+       :meth:`.AutomapBase.prepare` were used within a multi-threaded context
+       against other threads which  may call :func:`.configure_mappers` as a
+       result of use of other mappers.  The unfinished mapping work of automap
+       is particularly sensitive to being pulled in by a
+       :func:`.configure_mappers` step leading to errors.

diff --git a/lib/sqlalchemy/ext/automap.py b/lib/sqlalchemy/ext/automap.py
index 30f48fcea178bce0a452da640cd4b5b7b8e2a107..cafb3d61c873444b985f81de48fdd346bb4efaaa 100644 (file)
--- a/lib/sqlalchemy/ext/automap.py
+++ b/lib/sqlalchemy/ext/automap.py
@@ -518,6 +518,7 @@ from .declarative.base import _DeferredMapperConfig
 from ..sql import and_
 from ..schema import ForeignKeyConstraint
 from ..orm import relationship, backref, interfaces
+from ..orm.mapper import _CONFIGURE_MUTEX
 from .. import util
 
 
@@ -754,49 +755,54 @@ class AutomapBase(object):
                 autoload_replace=False
             )
 
-        table_to_map_config = dict(
-            (m.local_table, m)
-            for m in _DeferredMapperConfig.
-            classes_for_base(cls, sort=False)
-        )
-
-        many_to_many = []
+        _CONFIGURE_MUTEX.acquire()
+        try:
+            table_to_map_config = dict(
+                (m.local_table, m)
+                for m in _DeferredMapperConfig.
+                classes_for_base(cls, sort=False)
+            )
 
-        for table in cls.metadata.tables.values():
-            lcl_m2m, rem_m2m, m2m_const = _is_many_to_many(cls, table)
-            if lcl_m2m is not None:
-                many_to_many.append((lcl_m2m, rem_m2m, m2m_const, table))
-            elif not table.primary_key:
-                continue
-            elif table not in table_to_map_config:
-                mapped_cls = type(
-                    classname_for_table(cls, table.name, table),
-                    (cls, ),
-                    {"__table__": table}
-                )
-                map_config = _DeferredMapperConfig.config_for_cls(mapped_cls)
-                cls.classes[map_config.cls.__name__] = mapped_cls
-                table_to_map_config[table] = map_config
-
-        for map_config in table_to_map_config.values():
-            _relationships_for_fks(cls,
-                                   map_config,
-                                   table_to_map_config,
-                                   collection_class,
-                                   name_for_scalar_relationship,
-                                   name_for_collection_relationship,
-                                   generate_relationship)
-
-        for lcl_m2m, rem_m2m, m2m_const, table in many_to_many:
-            _m2m_relationship(cls, lcl_m2m, rem_m2m, m2m_const, table,
-                              table_to_map_config,
-                              collection_class,
-                              name_for_scalar_relationship,
-                              name_for_collection_relationship,
-                              generate_relationship)
-
-        for map_config in _DeferredMapperConfig.classes_for_base(cls):
-            map_config.map()
+            many_to_many = []
+
+            for table in cls.metadata.tables.values():
+                lcl_m2m, rem_m2m, m2m_const = _is_many_to_many(cls, table)
+                if lcl_m2m is not None:
+                    many_to_many.append((lcl_m2m, rem_m2m, m2m_const, table))
+                elif not table.primary_key:
+                    continue
+                elif table not in table_to_map_config:
+                    mapped_cls = type(
+                        classname_for_table(cls, table.name, table),
+                        (cls, ),
+                        {"__table__": table}
+                    )
+                    map_config = _DeferredMapperConfig.config_for_cls(
+                        mapped_cls)
+                    cls.classes[map_config.cls.__name__] = mapped_cls
+                    table_to_map_config[table] = map_config
+
+            for map_config in table_to_map_config.values():
+                _relationships_for_fks(cls,
+                                       map_config,
+                                       table_to_map_config,
+                                       collection_class,
+                                       name_for_scalar_relationship,
+                                       name_for_collection_relationship,
+                                       generate_relationship)
+
+            for lcl_m2m, rem_m2m, m2m_const, table in many_to_many:
+                _m2m_relationship(cls, lcl_m2m, rem_m2m, m2m_const, table,
+                                  table_to_map_config,
+                                  collection_class,
+                                  name_for_scalar_relationship,
+                                  name_for_collection_relationship,
+                                  generate_relationship)
+
+            for map_config in _DeferredMapperConfig.classes_for_base(cls):
+                map_config.map()
+        finally:
+            _CONFIGURE_MUTEX.release()
 
     _sa_decl_prepare = True
     """Indicate that the mapping of classes should be deferred.

diff --git a/test/ext/test_automap.py b/test/ext/test_automap.py
index 768f5ed0f2e9c8e10b82faa1753398d93f3f3eb2..4ac0860c887369b2eacd68166f6372a381968774 100644 (file)
--- a/test/ext/test_automap.py
+++ b/test/ext/test_automap.py
@@ -4,10 +4,15 @@ from sqlalchemy.ext.automap import automap_base
 from sqlalchemy.orm import relationship, interfaces, configure_mappers
 from sqlalchemy.ext.automap import generate_relationship
 from sqlalchemy.testing.mock import Mock, patch
-from sqlalchemy import String, Integer, ForeignKey
+from sqlalchemy import String, Integer, ForeignKey, MetaData
 from sqlalchemy import testing
 from sqlalchemy.testing.schema import Table, Column
 
+from sqlalchemy import create_engine
+import threading
+import time
+import random
+
 
 class AutomapTest(fixtures.MappedTest):
     @classmethod
@@ -339,3 +344,56 @@ class AutomapInhTest(fixtures.MappedTest):
         Base.prepare(
             engine=testing.db, reflect=True,
             generate_relationship=_gen_relationship)
+
+
+class ConcurrentAutomapTest(fixtures.TestBase):
+    __only_on__ = 'sqlite'
+
+    def _make_tables(self, e):
+        m = MetaData()
+        for i in range(15):
+            Table(
+                'table_%d' % i,
+                m,
+                Column('id', Integer, primary_key=True),
+                Column('data', String(50)),
+                Column(
+                    't_%d_id' % (i - 1),
+                    ForeignKey('table_%d.id' % (i - 1))
+                ) if i > 4 else None
+            )
+        m.drop_all(e)
+        m.create_all(e)
+
+    def _automap(self, e):
+        Base = automap_base()
+
+        Base.prepare(e, reflect=True)
+
+        time.sleep(.01)
+        configure_mappers()
+
+    def _chaos(self):
+        e = create_engine("sqlite://")
+        try:
+            self._make_tables(e)
+            for i in range(2):
+                try:
+                    self._automap(e)
+                except:
+                    self._success = False
+                    raise
+                time.sleep(random.random())
+        finally:
+            e.dispose()
+
+    def test_concurrent_automaps_w_configure(self):
+        self._success = True
+        threads = [threading.Thread(target=self._chaos) for i in range(30)]
+        for t in threads:
+            t.start()
+
+        for t in threads:
+            t.join()
+
+        assert self._success, "One or more threads failed"
\ No newline at end of file