From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 28 Sep 2020 02:40:09 +0000 (-0400)
Subject: dont use uninstrument event to dispose registry entry
X-Git-Tag: rel_1_4_0b1~71^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cda023bbe391d175c80c4d82196538b42575d937;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

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 13626fb217..7f2c61a05c 100644
--- 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 41cc881123..70fffa2951 100644
--- 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 b9c890429e..685824dda4 100644
--- 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 0349c445ce..29a509cb9a 100644
--- 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 821c8a3c89..296ddf385d 100644
--- 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):