WeakCompositeKey was coded incorrectly and was not weakly referencing anything. ...

diff --git a/CHANGES b/CHANGES
index ac9214143463f2a2fefa7acae7e65cec484bcddd..0feb2fdc2d195182e52bb1461c2385373f17cd22 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -8,6 +8,10 @@ CHANGES
 ========
 
 - orm
+    - Removed an internal join cache which could potentially leak 
+      memory when issuing query.join() repeatedly to ad-hoc 
+      selectables.
+  
     - Modernized the "no mapped table" exception and added a more
       explicit __table__/__tablename__ exception to declarative.
 

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 7198cc830fe0e2db36fc7a9de0891f8e978a86f9..c83e03599d08fd3e6eefb4f8b0cd5c52abcfc9b7 100644 (file)
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -381,7 +381,6 @@ class RelationProperty(StrategizedProperty):
         self.join_depth = join_depth
         self.local_remote_pairs = _local_remote_pairs
         self.extension = extension
-        self.__join_cache = {}
         self.comparator_factory = comparator_factory or RelationProperty.Comparator
         self.comparator = self.comparator_factory(self, None)
         util.set_creation_order(self)
@@ -1004,12 +1003,6 @@ class RelationProperty(StrategizedProperty):
         return self.mapper.common_parent(self.parent)
 
     def _create_joins(self, source_polymorphic=False, source_selectable=None, dest_polymorphic=False, dest_selectable=None, of_type=None):
-        key = util.WeakCompositeKey(source_polymorphic, source_selectable, dest_polymorphic, dest_selectable, of_type)
-        try:
-            return self.__join_cache[key]
-        except KeyError:
-            pass
-
         if source_selectable is None:
             if source_polymorphic and self.parent.with_polymorphic:
                 source_selectable = self.parent._with_polymorphic_selectable
@@ -1076,10 +1069,9 @@ class RelationProperty(StrategizedProperty):
         else:
             target_adapter = None
 
-        self.__join_cache[key] = ret = (primaryjoin, secondaryjoin, 
+        return (primaryjoin, secondaryjoin, 
                 (source_selectable or self.parent.local_table), 
                 (dest_selectable or self.mapper.local_table), secondary, target_adapter)
-        return ret
 
     def _get_join(self, parent, primary=True, secondary=True, polymorphic_parent=True):
         """deprecated.  use primary_join_against(), secondary_join_against(), full_join_against()"""

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 5f820565f128f9890b4375d2a468d8374f292145..7195310cdf0ab9db0943876d332d91043a0552fa 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -594,7 +594,6 @@ class EagerLoader(AbstractRelationLoader):
     
     def init(self):
         super(EagerLoader, self).init()
-        self.clauses = {}
         self.join_depth = self.parent_property.join_depth
 
     def init_class_attribute(self):
@@ -669,14 +668,8 @@ class EagerLoader(AbstractRelationLoader):
     
         towrap = context.eager_joins.setdefault(entity_key, default_towrap)
     
-        # create AliasedClauses object to build up the eager query.  this is cached after 1st creation.
-        # this also allows ORMJoin to cache the aliased joins it produces since we pass the same
-        # args each time in the typical case.
-        path_key = util.WeakCompositeKey(*path)
-        try:
-            clauses = self.clauses[path_key]
-        except KeyError:
-            self.clauses[path_key] = clauses = mapperutil.ORMAdapter(mapperutil.AliasedClass(self.mapper), 
+        # create AliasedClauses object to build up the eager query.  
+        clauses = mapperutil.ORMAdapter(mapperutil.AliasedClass(self.mapper), 
                     equivalents=self.mapper._equivalent_columns)
 
         if adapter:

diff --git a/lib/sqlalchemy/util.py b/lib/sqlalchemy/util.py
index 619888135df487529947d4207d44796a96f328a6..110ef21d5ef6bf027e45ea5e55536d783cd8a244 100644 (file)
--- a/lib/sqlalchemy/util.py
+++ b/lib/sqlalchemy/util.py
@@ -1178,38 +1178,6 @@ class _TLocalRegistry(ScopedRegistry):
         except AttributeError:
             pass
 
-class WeakCompositeKey(object):
-    """an weak-referencable, hashable collection which is strongly referenced
-    until any one of its members is garbage collected.
-
-    """
-    keys = set()
-
-    __slots__ = 'args', '__weakref__'
-    
-    def __init__(self, *args):
-        self.args = [self.__ref(arg) for arg in args]
-        WeakCompositeKey.keys.add(self)
-
-    def __ref(self, arg):
-        if isinstance(arg, type):
-            return weakref.ref(arg, self.__remover)
-        else:
-            return lambda: arg
-
-    def __remover(self, wr):
-        WeakCompositeKey.keys.discard(self)
-
-    def __hash__(self):
-        return hash(tuple(self))
-
-    def __cmp__(self, other):
-        return cmp(tuple(self), tuple(other))
-
-    def __iter__(self):
-        return iter(arg() for arg in self.args)
-
-
 class _symbol(object):
     def __init__(self, name):
         """Construct a new named symbol."""

diff --git a/test/base/utils.py b/test/base/utils.py
index b3e11f230b394c8551471d91b55fb29e79f9b93f..0b7762b7d6b81eb55c37ed9d642e38a97c802d93 100644 (file)
--- a/test/base/utils.py
+++ b/test/base/utils.py
@@ -4,7 +4,6 @@ from sqlalchemy import util, sql, exc
 from testlib import TestBase
 from testlib.testing import eq_, is_, ne_
 
-
 class OrderedDictTest(TestBase):
     def test_odict(self):
         o = util.OrderedDict()
@@ -943,5 +942,6 @@ class TestClassHierarchy(TestBase):
         eq_(set(util.class_hierarchy(Mixin)), set())
         eq_(set(util.class_hierarchy(A)), set((A, B, object)))
 
+        
 if __name__ == "__main__":
     testenv.main()

diff --git a/test/profiling/memusage.py b/test/profiling/memusage.py
index ab6a5e2c0aca17df3588529ba67b60cfdcbc9339..3e5db8ae42abdaf6483810d89aa5f3645af9f555 100644 (file)
--- a/test/profiling/memusage.py
+++ b/test/profiling/memusage.py
@@ -300,6 +300,44 @@ class MemUsageTest(EnsureZeroed):
             metadata.drop_all()
         assert_no_mappers()
 
+    def test_join_cache(self):
+        metadata = MetaData(testing.db)
+
+        table1 = Table("table1", metadata,
+            Column('id', Integer, primary_key=True),
+            Column('data', String(30))
+            )
+
+        table2 = Table("table2", metadata,
+            Column('id', Integer, primary_key=True),
+            Column('data', String(30)),
+            Column('t1id', Integer, ForeignKey('table1.id'))
+            )
+        
+        class Foo(object):
+            pass
+            
+        class Bar(object):
+            pass
+            
+        mapper(Foo, table1, properties={
+            'bars':relation(mapper(Bar, table2))
+        })
+        metadata.create_all()
+
+        session = sessionmaker()
+        
+        @profile_memory
+        def go():
+            s = table2.select()
+            session().query(Foo).join((s, Foo.bars)).all()
+            
+        try:
+            go()
+        finally:
+            metadata.drop_all()
+            
+            
     def test_mutable_identity(self):
         metadata = MetaData(testing.db)