Mutex the declarative scan/map process against configure_mappers()

Applied the mapper "configure mutex" against the declarative class mapping
process, to guard against the race which can occur if mappers are used
while dynamic module import schemes are still in the process of configuring
mappers for related classes.  This does not guard against all possible race
conditions, such as if the concurrent import has not yet encountered the
dependent classes as of yet, however it guards against as much as possible
within the SQLAlchemy declarative process.

Fixes: #4686
Change-Id: I0349036b8078bd42265ab40862cfbfe5bf9d5b44

diff --git a/doc/build/changelog/unreleased_13/4686.rst b/doc/build/changelog/unreleased_13/4686.rst
new file mode 100644 (file)
index 0000000..bf1f803
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4686.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4686
+
+    Applied the mapper "configure mutex" against the declarative class mapping
+    process, to guard against the race which can occur if mappers are used
+    while dynamic module import schemes are still in the process of configuring
+    mappers for related classes.  This does not guard against all possible race
+    conditions, such as if the concurrent import has not yet encountered the
+    dependent classes as of yet, however it guards against as much as possible
+    within the SQLAlchemy declarative process.

diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index 62db282d1fb225a73cb6d33814658f20adab5443..622a837363c0163016cafe1d80892d2ff5ea47df 100644 (file)
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -17,6 +17,7 @@ from ... import util
 from ...orm import class_mapper
 from ...orm import exc as orm_exc
 from ...orm import mapper
+from ...orm import mapperlib
 from ...orm import synonym
 from ...orm.attributes import QueryableAttribute
 from ...orm.base import _is_mapped_class
@@ -155,6 +156,7 @@ class _MapperConfig(object):
             cfg_cls = _DeferredMapperConfig
         else:
             cfg_cls = _MapperConfig
+
         cfg_cls(cls_, classname, dict_)
 
     def __init__(self, cls_, classname, dict_):
@@ -177,17 +179,21 @@ class _MapperConfig(object):
 
         self._scan_attributes()
 
-        clsregistry.add_class(self.classname, self.cls)
+        mapperlib._CONFIGURE_MUTEX.acquire()
+        try:
+            clsregistry.add_class(self.classname, self.cls)
 
-        self._extract_mappable_attributes()
+            self._extract_mappable_attributes()
 
-        self._extract_declared_columns()
+            self._extract_declared_columns()
 
-        self._setup_table()
+            self._setup_table()
 
-        self._setup_inheritance()
+            self._setup_inheritance()
 
-        self._early_mapping()
+            self._early_mapping()
+        finally:
+            mapperlib._CONFIGURE_MUTEX.release()
 
     def _early_mapping(self):
         self.map()

diff --git a/test/ext/declarative/test_concurrency.py b/test/ext/declarative/test_concurrency.py
new file mode 100644 (file)
index 0000000..b73b65e
--- /dev/null
+++ b/test/ext/declarative/test_concurrency.py
@@ -0,0 +1,78 @@
+import random
+import threading
+import time
+
+from sqlalchemy import Column
+from sqlalchemy import ForeignKey
+from sqlalchemy import Integer
+from sqlalchemy import String
+from sqlalchemy.ext.declarative import declarative_base
+from sqlalchemy.ext.declarative import declared_attr
+from sqlalchemy.orm import clear_mappers
+from sqlalchemy.orm import relationship
+from sqlalchemy.orm import Session
+from sqlalchemy.testing import fixtures
+
+
+class ConcurrentUseDeclMappingTest(fixtures.TestBase):
+    @classmethod
+    def make_a(cls, Base):
+        class A(Base):
+            __tablename__ = "a"
+
+            id = Column(Integer, primary_key=True)
+            data = Column(String)
+            bs = relationship("B")
+
+        # need a strong ref so that the class is not gc'ed
+        cls.A = A
+
+    @classmethod
+    def query_a(cls, Base, result):
+        s = Session()
+        time.sleep(random.random() / 100)
+        A = cls.A
+        try:
+            s.query(A).join(A.bs)
+        except Exception as err:
+            result[0] = err
+            print(err)
+        else:
+            result[0] = True
+            print("worked")
+
+    @classmethod
+    def make_b(cls, Base):
+        class B(Base):
+            __tablename__ = "b"
+            id = Column(Integer, primary_key=True)
+
+            @declared_attr
+            def data(cls):
+                time.sleep(0.001)
+                return Column(String)
+
+            a_id = Column(ForeignKey("a.id"))
+
+        cls.B = B
+
+    def test_concurrent_create(self):
+        for i in range(50):
+            Base = declarative_base()
+            clear_mappers()
+
+            self.make_a(Base)
+            result = [False]
+            threads = [
+                threading.Thread(target=self.make_b, args=(Base,)),
+                threading.Thread(target=self.query_a, args=(Base, result)),
+            ]
+
+            for t in threads:
+                t.start()
+
+            for t in threads:
+                t.join()
+
+            if isinstance(result[0], Exception):
+                raise result[0]