- Fixed an extremely unlikely memory issue where when using

:class:`.DeferredReflection`
to define classes pending for reflection, if some subset of those
classes were discarded before the :meth:`.DeferredReflection.prepare`
method were called to reflect and map the class, a strong reference
to the class would remain held within the declarative internals.
This internal collection of "classes to map" now uses weak
references against the classes themselves.

4 files changed, 93 insertions, 21 deletions

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 7baf0ad43..c72d853d9 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -15,6 +15,18 @@
     :version: 0.9.1
 
     .. change::
+        :tags: bug, orm, declarative
+
+        Fixed an extremely unlikely memory issue where when using
+        :class:`.DeferredReflection`
+        to define classes pending for reflection, if some subset of those
+        classes were discarded before the :meth:`.DeferredReflection.prepare`
+        method were called to reflect and map the class, a strong reference
+        to the class would remain held within the declarative internals.
+        This internal collection of "classes to map" now uses weak
+        references against the classes themselves.
+
+    .. change::
         :tags: bug, orm
         :pullreq: bitbucket:9
 
diff --git a/lib/sqlalchemy/ext/declarative/api.py b/lib/sqlalchemy/ext/declarative/api.py
index 64bf7fd9f..c9b5a9195 100644
--- a/lib/sqlalchemy/ext/declarative/api.py
+++ b/lib/sqlalchemy/ext/declarative/api.py
@@ -18,7 +18,7 @@ import weakref
 
 from .base import _as_declarative, \
                 _declarative_constructor,\
-                _MapperConfig, _add_attribute
+                _DeferredMapperConfig, _add_attribute
 from .clsregistry import _class_resolver
 
 
@@ -468,8 +468,7 @@ class DeferredReflection(object):
         """Reflect all :class:`.Table` objects for all current
         :class:`.DeferredReflection` subclasses"""
 
-        to_map = [m for m in _MapperConfig.configs.values()
-                    if issubclass(m.cls, cls)]
+        to_map = _DeferredMapperConfig.classes_for_base(cls)
         for thingy in to_map:
             cls._sa_decl_prepare(thingy.local_table, engine)
             thingy.map()
@@ -479,7 +478,7 @@ class DeferredReflection(object):
                 if isinstance(rel, properties.RelationshipProperty) and \
                     rel.secondary is not None:
                     if isinstance(rel.secondary, Table):
-                        cls._sa_decl_prepare(rel.secondary, engine)
+                        cls._reflect_table(rel.secondary, engine)
                     elif isinstance(rel.secondary, _class_resolver):
                         rel.secondary._resolvers += (
                             cls._sa_deferred_table_resolver(engine, metadata),
@@ -489,7 +488,7 @@ class DeferredReflection(object):
     def _sa_deferred_table_resolver(cls, engine, metadata):
         def _resolve(key):
             t1 = Table(key, metadata)
-            cls._sa_decl_prepare(t1, engine)
+            cls._reflect_table(t1, engine)
             return t1
         return _resolve
 
@@ -500,10 +499,14 @@ class DeferredReflection(object):
         # will fill in db-loaded columns
         # into the existing Table object.
         if local_table is not None:
-            Table(local_table.name,
-                local_table.metadata,
-                extend_existing=True,
-                autoload_replace=False,
-                autoload=True,
-                autoload_with=engine,
-                schema=local_table.schema)
+            cls._reflect_table(local_table, engine)
+
+    @classmethod
+    def _reflect_table(cls, table, engine):
+        Table(table.name,
+            table.metadata,
+            extend_existing=True,
+            autoload_replace=False,
+            autoload=True,
+            autoload_with=engine,
+            schema=table.schema)
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index 69e4b9eea..3cd600980 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -16,11 +16,12 @@ from ...sql import expression
 from ... import event
 from . import clsregistry
 import collections
+import weakref
 
 def _declared_mapping_info(cls):
     # deferred mapping
-    if cls in _MapperConfig.configs:
-        return _MapperConfig.configs[cls]
+    if _DeferredMapperConfig.has_cls(cls):
+        return _DeferredMapperConfig.config_for_cls(cls)
     # regular mapping
     elif _is_mapped_class(cls):
         return class_mapper(cls, configure=False)
@@ -304,19 +305,24 @@ def _as_declarative(cls, classname, dict_):
                     inherited_mapped_table is not inherited_table:
                     inherited_mapped_table._refresh_for_new_column(c)
 
-    mt = _MapperConfig(mapper_cls,
+    defer_map = hasattr(cls, '_sa_decl_prepare')
+    if defer_map:
+        cfg_cls = _DeferredMapperConfig
+    else:
+        cfg_cls = _MapperConfig
+    mt = cfg_cls(mapper_cls,
                        cls, table,
                        inherits,
                        declared_columns,
                        column_copies,
                        our_stuff,
                        mapper_args_fn)
-    if not hasattr(cls, '_sa_decl_prepare'):
+    if not defer_map:
         mt.map()
 
 
 class _MapperConfig(object):
-    configs = util.OrderedDict()
+
     mapped_table = None
 
     def __init__(self, mapper_cls,
@@ -334,7 +340,7 @@ class _MapperConfig(object):
         self.mapper_args_fn = mapper_args_fn
         self.declared_columns = declared_columns
         self.column_copies = column_copies
-        self.configs[cls] = self
+
 
     def _prepare_mapper_arguments(self):
         properties = self.properties
@@ -391,7 +397,6 @@ class _MapperConfig(object):
         return result_mapper_args
 
     def map(self):
-        self.configs.pop(self.cls, None)
         mapper_args = self._prepare_mapper_arguments()
         self.cls.__mapper__ = self.mapper_cls(
             self.cls,
@@ -399,6 +404,42 @@ class _MapperConfig(object):
             **mapper_args
         )
 
+class _DeferredMapperConfig(_MapperConfig):
+    _configs = util.OrderedDict()
+
+    @property
+    def cls(self):
+        return self._cls()
+
+    @cls.setter
+    def cls(self, class_):
+        self._cls = weakref.ref(class_, self._remove_config_cls)
+        self._configs[self._cls] = self
+
+    @classmethod
+    def _remove_config_cls(cls, ref):
+        cls._configs.pop(ref, None)
+
+    @classmethod
+    def has_cls(cls, class_):
+        # 2.6 fails on weakref if class_ is an old style class
+        return isinstance(class_, type) and \
+                weakref.ref(class_) in cls._configs
+
+    @classmethod
+    def config_for_cls(cls, class_):
+        return cls._configs[weakref.ref(class_)]
+
+
+    @classmethod
+    def classes_for_base(cls, base_cls):
+        return [m for m in cls._configs.values()
+                    if issubclass(m.cls, base_cls)]
+
+    def map(self):
+        self._configs.pop(self._cls, None)
+        super(_DeferredMapperConfig, self).map()
+
 
 def _add_attribute(cls, key, value):
     """add an attribute to an existing declarative class.
diff --git a/test/ext/declarative/test_reflection.py b/test/ext/declarative/test_reflection.py
index 26496f1ad..f4bda6995 100644
--- a/test/ext/declarative/test_reflection.py
+++ b/test/ext/declarative/test_reflection.py
@@ -7,6 +7,8 @@ from sqlalchemy.orm import relationship, create_session, \
     clear_mappers, \
     Session
 from sqlalchemy.testing import fixtures
+from sqlalchemy.testing.util import gc_collect
+from sqlalchemy.ext.declarative.base import _DeferredMapperConfig
 
 class DeclarativeReflectionBase(fixtures.TablesTest):
     __requires__ = 'reflectable_autoincrement',
@@ -147,8 +149,7 @@ class DeclarativeReflectionTest(DeclarativeReflectionBase):
 class DeferredReflectBase(DeclarativeReflectionBase):
     def teardown(self):
         super(DeferredReflectBase, self).teardown()
-        from sqlalchemy.ext.declarative.base import _MapperConfig
-        _MapperConfig.configs.clear()
+        _DeferredMapperConfig._configs.clear()
 
 Base = None
 
@@ -292,6 +293,21 @@ class DeferredReflectionTest(DeferredReflectBase):
             ]
         )
 
+    @testing.requires.predictable_gc
+    def test_cls_not_strong_ref(self):
+        class User(decl.DeferredReflection, fixtures.ComparableEntity,
+                            Base):
+            __tablename__ = 'users'
+        class Address(decl.DeferredReflection, fixtures.ComparableEntity,
+                            Base):
+            __tablename__ = 'addresses'
+        eq_(len(_DeferredMapperConfig._configs), 2)
+        del Address
+        gc_collect()
+        eq_(len(_DeferredMapperConfig._configs), 1)
+        decl.DeferredReflection.prepare(testing.db)
+        assert not _DeferredMapperConfig._configs
+
 class DeferredSecondaryReflectionTest(DeferredReflectBase):
     @classmethod
     def define_tables(cls, metadata):