]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Fixed potential memory leak which could occur if an
authorMike Bayer <mike_mp@zzzcomputing.com>
Sat, 12 Jan 2013 22:32:32 +0000 (17:32 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sat, 12 Jan 2013 22:32:32 +0000 (17:32 -0500)
arbitrary number of :func:`.sessionmaker` objects
were created.   The anonymous subclass created by
the sessionmaker, when dereferenced, would not be garbage
collected due to remaining class-level references from the
event package.  This issue also applies to any custom system
that made use of ad-hoc subclasses in conjunction with
an event dispatcher. [ticket:2650]

doc/build/changelog/changelog_07.rst
lib/sqlalchemy/event.py
test/aaa_profiling/test_memusage.py
test/base/test_events.py

index fdf5c1fa28329e05989487f796c42e106476d874..aebe92bc5e1e3d648dee13e300281cb7afbf3a02 100644 (file)
       to the MSSQL dialect's "schema rendering"
       logic's failure to take .key into account.
 
+    .. change::
+        :tags: orm, bug
+        :tickets: 2650
+
+      Fixed potential memory leak which could occur if an
+      arbitrary number of :func:`.sessionmaker` objects
+      were created.   The anonymous subclass created by
+      the sessionmaker, when dereferenced, would not be garbage
+      collected due to remaining class-level references from the
+      event package.  This issue also applies to any custom system
+      that made use of ad-hoc subclasses in conjunction with
+      an event dispatcher.
+
     .. change::
         :tags: orm, bug
         :tickets: 2640
index 4e7484326d0e0e2dbf11291c4fe60dcc649abd80..dabebb819879da6a0353edcf974d82d61c574edd 100644 (file)
@@ -7,6 +7,7 @@
 """Base event API."""
 
 from sqlalchemy import util, exc
+import weakref
 
 CANCEL = util.symbol('CANCEL')
 NO_RETVAL = util.symbol('NO_RETVAL')
@@ -203,11 +204,12 @@ class _DispatchDescriptor(object):
     def __init__(self, fn):
         self.__name__ = fn.__name__
         self.__doc__ = fn.__doc__
-        self._clslevel = util.defaultdict(list)
-        self._empty_listeners = {}
+        self._clslevel = weakref.WeakKeyDictionary()
+        self._empty_listeners = weakref.WeakKeyDictionary()
 
     def _contains(self, cls, evt):
-        return evt in self._clslevel[cls]
+        return cls in self._clslevel and \
+            evt in self._clslevel[cls]
 
     def insert(self, obj, target, propagate):
         assert isinstance(target, type), \
@@ -219,6 +221,8 @@ class _DispatchDescriptor(object):
             if cls is not target and cls not in self._clslevel:
                 self.update_subclass(cls)
             else:
+                if cls not in self._clslevel:
+                    self._clslevel[cls] = []
                 self._clslevel[cls].insert(0, obj)
 
     def append(self, obj, target, propagate):
@@ -232,9 +236,13 @@ class _DispatchDescriptor(object):
             if cls is not target and cls not in self._clslevel:
                 self.update_subclass(cls)
             else:
+                if cls not in self._clslevel:
+                    self._clslevel[cls] = []
                 self._clslevel[cls].append(obj)
 
     def update_subclass(self, target):
+        if target not in self._clslevel:
+            self._clslevel[target] = []
         clslevel = self._clslevel[target]
         for cls in target.__mro__[1:]:
             if cls in self._clslevel:
@@ -249,7 +257,8 @@ class _DispatchDescriptor(object):
         while stack:
             cls = stack.pop(0)
             stack.extend(cls.__subclasses__())
-            self._clslevel[cls].remove(obj)
+            if cls in self._clslevel:
+                self._clslevel[cls].remove(obj)
 
     def clear(self):
         """Clear all class level listeners"""
index 3120683b354406d15139442504cfcffc4ea4eefc..722b0556746895f1681ba817c1f65be642448e8c 100644 (file)
@@ -1,12 +1,12 @@
 from test.lib.testing import eq_
 from sqlalchemy.orm import mapper, relationship, create_session, \
-    clear_mappers, sessionmaker, class_mapper
+    clear_mappers, sessionmaker
 from sqlalchemy.orm.mapper import _mapper_registry
 from sqlalchemy.orm.session import _sessions
 import operator
 from test.lib import testing, engines
 from sqlalchemy import MetaData, Integer, String, ForeignKey, \
-    PickleType, create_engine, Unicode
+    PickleType, Unicode, select
 from test.lib.schema import Table, Column
 import sqlalchemy as sa
 from sqlalchemy.sql import column
@@ -27,13 +27,21 @@ def profile_memory(func):
     # run the test 50 times.  if length of gc.get_objects()
     # keeps growing, assert false
 
+    def get_objects_skipping_sqlite_issue():
+        # pysqlite keeps adding weakref objects which only
+        # get reset after 220 iterations, which is too long
+        # to run lots of these tests, so just filter them
+        # out.
+        return [o for o in gc.get_objects()
+                if not isinstance(o, weakref.ref)]
+
     def profile(*args):
         gc_collect()
         samples = [0 for x in range(0, 50)]
         for x in range(0, 50):
             func(*args)
             gc_collect()
-            samples[x] = len(gc.get_objects())
+            samples[x] = len(get_objects_skipping_sqlite_issue())
 
         print "sample gc sizes:", samples
 
@@ -139,6 +147,18 @@ class MemUsageTest(EnsureZeroed):
         del m1, m2, m3
         assert_no_mappers()
 
+    def test_sessionmaker(self):
+        @profile_memory
+        def go():
+            sessmaker = sessionmaker(bind=testing.db)
+            sess = sessmaker()
+            r = sess.execute(select([1]))
+            r.close()
+            sess.close()
+            del sess
+            del sessmaker
+        go()
+
     @testing.crashes('sqlite', ':memory: connection not suitable here')
     def test_orm_many_engines(self):
         metadata = MetaData(testing.db)
@@ -282,11 +302,6 @@ class MemUsageTest(EnsureZeroed):
         finally:
             metadata.drop_all()
 
-    @testing.fails_if(lambda : testing.db.dialect.name == 'sqlite' \
-                      and testing.db.dialect.dbapi.version_info >= (2,
-                      5),
-                      'Newer pysqlites generate warnings here too and '
-                      'have similar issues.')
     def test_unicode_warnings(self):
         metadata = MetaData(testing.db)
         table1 = Table('mytable', metadata, Column('col1', Integer,
@@ -496,8 +511,6 @@ class MemUsageTest(EnsureZeroed):
             metadata.drop_all()
         assert_no_mappers()
 
-    @testing.fails_if(lambda : testing.db.dialect.name == 'sqlite' \
-                      and testing.db.dialect.dbapi.version > '2.5')
     @testing.provide_metadata
     def test_key_fallback_result(self):
         e = testing.db
@@ -516,8 +529,6 @@ class MemUsageTest(EnsureZeroed):
     # in pysqlite itself. background at:
     # http://thread.gmane.org/gmane.comp.python.db.pysqlite.user/2290
 
-    @testing.fails_if(lambda : testing.db.dialect.name == 'sqlite' \
-                      and testing.db.dialect.dbapi.version > '2.5')
     def test_join_cache(self):
         metadata = MetaData(testing.db)
         table1 = Table('table1', metadata, Column('id', Integer,
index 9d1fa21009fab284b391d4faabf55412477be46c..eb98560cd41669986e799dbbf2e9f29ff883945f 100644 (file)
@@ -2,8 +2,9 @@
 
 from test.lib.testing import eq_, assert_raises, assert_raises_message, \
     is_, is_not_
-from sqlalchemy import event, exc, util
+from sqlalchemy import event, exc
 from test.lib import fixtures
+from test.lib.util import gc_collect
 
 class TestEvents(fixtures.TestBase):
     """Test class- and instance-level event registration."""
@@ -359,6 +360,31 @@ class TestCustomTargets(fixtures.TestBase):
             listen, "event_one", self.Target
         )
 
+class SubclassGrowthTest(fixtures.TestBase):
+    """test that ad-hoc subclasses are garbage collected."""
+
+    def setUp(self):
+        class TargetEvents(event.Events):
+            def some_event(self, x, y):
+                pass
+
+        class Target(object):
+            dispatch = event.dispatcher(TargetEvents)
+
+        self.Target = Target
+
+    def test_subclass(self):
+        class SubTarget(self.Target):
+            pass
+
+        st = SubTarget()
+        st.dispatch.some_event(1, 2)
+        del st
+        del SubTarget
+        gc_collect()
+        eq_(self.Target.__subclasses__(), [])
+
+
 class TestListenOverride(fixtures.TestBase):
     """Test custom listen functions which change the listener function signature."""