From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 16 May 2019 15:26:04 +0000 (-0400)
Subject: Mutex the declarative scan/map process against configure_mappers()
X-Git-Tag: rel_1_4_0b1~872^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5039c6f01d0bd1f58f950e80cddf7472444a70a4;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

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
index 0000000000..bf1f80311e
--- /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 62db282d1f..622a837363 100644
--- 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
index 0000000000..b73b65ec29
--- /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]