Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

automap.prepare() creates classes that are referenced but not mapped, causes errors under concurrency #4266

Closed
sqlalchemy-bot opened this issue May 27, 2018 · 4 comments
Labels
bug Something isn't working sqlalchemy.ext extension modules, most of which are ORM related
Milestone

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by Michael Bayer (@zzzeek)

demo:

from sqlalchemy import *
from sqlalchemy.orm import configure_mappers
from sqlalchemy.ext.automap import automap_base
import time
import random
import threading


def make_tables(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(e):
    id_ = threading.get_ident()
    Base = automap_base()

    Base.prepare(e, reflect=True)
    print("thread %s prepared!" % id_)

    time.sleep(.01)
    configure_mappers()
    print("thread %s we're good!" % id_)


def chaos():
    while True:
        automap(e)
        time.sleep(random.random())

e = create_engine("mysql://scott:tiger@localhost/test")

make_tables(e)

threads = [threading.Thread(target=chaos) for i in range(30)]
for t in threads:
    t.start()

for t in threads:
    t.join()

output:

thread 139667543086848 prepared!
thread 139667526301440 prepared!
thread 139667559872256 prepared!
Exception in thread Thread-5:
Traceback (most recent call last):
  File "/usr/lib64/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib64/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "test.py", line 40, in chaos
    automap(e)
  File "test.py", line 34, in automap
    configure_mappers()
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/mapper.py", line 3026, in configure_mappers
    raise e
sqlalchemy.exc.InvalidRequestError: One or more mappers failed to initialize - can't proceed with initialization of other mappers. Triggering mapper: 'Mapper|table_14|table_14'. Original exception was: Class 'sqlalchemy.ext.automap.table_13' is not mapped

Exception in thread Thread-3:
Traceback (most recent call last):
  File "/usr/lib64/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib64/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "test.py", line 40, in chaos
    automap(e)
  File "test.py", line 34, in automap
    configure_mappers()
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/mapper.py", line 3029, in configure_mappers
    mapper._post_configure_properties()
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/mapper.py", line 1828, in _post_configure_properties
    prop.init()
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/interfaces.py", line 184, in init
    self.do_init()
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/relationships.py", line 1655, in do_init
    self._process_dependent_arguments()
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/relationships.py", line 1712, in _process_dependent_arguments
    self.target = self.mapper.mapped_table
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/util/langhelpers.py", line 767, in __get__
    obj.__dict__[self.__name__] = result = self.fget(obj)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/relationships.py", line 1634, in mapper
    configure=False)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/base.py", line 426, in class_mapper
    raise exc.UnmappedClassError(class_)
sqlalchemy.orm.exc.UnmappedClassError: Class 'sqlalchemy.ext.automap.table_13' is not mapped

... continues until all threads are done

this can be patched as:

diff --git a/lib/sqlalchemy/ext/automap.py b/lib/sqlalchemy/ext/automap.py
index 30f48fcea..b2fdbf802 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,53 @@ 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.

@sqlalchemy-bot
Copy link
Collaborator Author

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

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

2ac7cad

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

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
(cherry picked from commit 2ac7cad)

553f67f

@sqlalchemy-bot sqlalchemy-bot added bug Something isn't working sqlalchemy.ext extension modules, most of which are ORM related labels Nov 27, 2018
@sqlalchemy-bot sqlalchemy-bot added this to the 1.2.x milestone Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sqlalchemy.ext extension modules, most of which are ORM related
Projects
None yet
Development

No branches or pull requests

1 participant