From: Mike Bayer Date: Sun, 27 May 2018 17:45:20 +0000 (-0400) Subject: Mutex on _CONFIGURE_MUTEX in automap.prepare() X-Git-Tag: rel_1_3_0b1~177^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ac7cad7179ff594d8bb3df9856c891bf4e51097;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..923c7c8516 --- /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 30f48fcea1..cafb3d61c8 100644 --- 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 768f5ed0f2..4ac0860c88 100644 --- 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