dont use uninstrument event to dispose registry entry

since 450f5c0d6519a439f4025c3892fe4c we've been seeing
errors during the uninstrument_class event where first
we saw an internal weakref being collected earlier than seen,
then fixing that we saw the listener collection changing during
iteration for similar reasons.

we would assume the issue is because of the interaction between
mapper / instrumentation/ registry during a test teardown
and the usage of the uninstrument_class event within this
interaction.   this interaction is too fundamental to be
relying upon this event in any case and when I wrote this
new code i was planning on changing that part in any case,
I just forgot.

Change-Id: I15744e01bb4d3349bfd529593ebd23eae658eaab

diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py
index 13626fb2179287a3d5123bbaf1b1d63754f93482..7f2c61a05c924e4a9ce4cb5ab26b798eebfdb35a 100644 (file)
--- a/lib/sqlalchemy/orm/__init__.py
+++ b/lib/sqlalchemy/orm/__init__.py
@@ -253,8 +253,13 @@ def clear_mappers():
     """
     with mapperlib._CONFIGURE_MUTEX:
         while _mapper_registry:
-            mapper, b = _mapper_registry.popitem()
-            mapper.dispose()
+            try:
+                mapper, b = _mapper_registry.popitem()
+            except KeyError:
+                # weak registry, item could have been collected
+                pass
+            else:
+                mapper.dispose()
 
 
 joinedload = strategy_options.joinedload._unbound_fn

diff --git a/lib/sqlalchemy/orm/decl_api.py b/lib/sqlalchemy/orm/decl_api.py
index 41cc8811237b1d9d8fcd88d72271f334c263a4c1..70fffa295139918a404e48d6cb838b9b60da6e38 100644 (file)
--- a/lib/sqlalchemy/orm/decl_api.py
+++ b/lib/sqlalchemy/orm/decl_api.py
@@ -454,7 +454,7 @@ class registry(object):
         self.metadata = lcl_metadata
         self.constructor = constructor
 
-    def _dispose_declarative_artifacts(self, cls):
+    def _dispose_cls(self, cls):
         clsregistry.remove_class(cls.__name__, cls, self._class_registry)
 
     def generate_base(

diff --git a/lib/sqlalchemy/orm/decl_base.py b/lib/sqlalchemy/orm/decl_base.py
index b9c890429e4f055ac607448b7d49536f6736da76..685824dda4656a878d3599c3491e76d430cd6a2a 100644 (file)
--- a/lib/sqlalchemy/orm/decl_base.py
+++ b/lib/sqlalchemy/orm/decl_base.py
@@ -189,12 +189,6 @@ class _MapperConfig(object):
             init_method=registry.constructor,
         )
 
-        event.listen(
-            cls_,
-            "class_uninstrument",
-            registry._dispose_declarative_artifacts,
-        )
-
     def set_cls_attribute(self, attrname, value):
 
         manager = instrumentation.manager_of_class(self.cls)

diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py
index 0349c445cea0c0360aa14ad8b45d06cdfe43fa1d..29a509cb9a7ed3bcfa501778ad7671e2f55a7f1d 100644 (file)
--- a/lib/sqlalchemy/orm/events.py
+++ b/lib/sqlalchemy/orm/events.py
@@ -67,6 +67,11 @@ class InstrumentationEvents(event.Events):
 
         def listen(target_cls, *arg):
             listen_cls = target()
+
+            # if weakref were collected, however this is not something
+            # that normally happens.   it was occurring during test teardown
+            # between mapper/registry/instrumentation_manager, however this
+            # interaction was changed to not rely upon the event system.
             if listen_cls is None:
                 return None
 

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 821c8a3c895ef1097f7874f41b189aff10461037..296ddf385d4077a3f8f068842346f7193dd72d3a 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1293,6 +1293,7 @@ class Mapper(
             and self.class_manager.is_mapped
             and self.class_manager.mapper is self
         ):
+            self.class_manager.registry._dispose_cls(self.class_)
             instrumentation.unregister_class(self.class_)
 
     def _configure_pks(self):