From 576224edd01f96bb505f9162282cfa9ba6efcb46 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 12 Jan 2013 17:21:35 -0500 Subject: [PATCH] - add workaround for sqlite memusage tests, so no longer need to count to 220/skip tests - Fixed potential memory leak which could occur if an arbitrary number of :class:`.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. Also in 0.7.10. [ticket:2650] --- doc/build/changelog/changelog_07.rst | 13 ++++++++++ doc/build/changelog/changelog_08.rst | 13 ++++++++++ lib/sqlalchemy/event.py | 17 ++++++++++--- test/aaa_profiling/test_memusage.py | 37 +++++++++++++++++++--------- test/base/test_events.py | 28 ++++++++++++++++++++- 5 files changed, 92 insertions(+), 16 deletions(-) diff --git a/doc/build/changelog/changelog_07.rst b/doc/build/changelog/changelog_07.rst index 71bf4e368c..b98ff0d834 100644 --- a/doc/build/changelog/changelog_07.rst +++ b/doc/build/changelog/changelog_07.rst @@ -8,6 +8,19 @@ :version: 0.7.10 :released: + .. change:: + :tags: orm, bug + :tickets: 2650 + + Fixed potential memory leak which could occur if an + arbitrary number of :class:`.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 diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst index 154e5fff6c..a45137167a 100644 --- a/doc/build/changelog/changelog_08.rst +++ b/doc/build/changelog/changelog_08.rst @@ -6,6 +6,19 @@ .. changelog:: :version: 0.8.0 + .. change:: + :tags: orm, bug + :tickets: 2650 + + Fixed potential memory leak which could occur if an + arbitrary number of :class:`.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. Also in 0.7.10. + .. change:: :tags: mssql, bug diff --git a/lib/sqlalchemy/event.py b/lib/sqlalchemy/event.py index 6293572a35..f28f19ee92 100644 --- a/lib/sqlalchemy/event.py +++ b/lib/sqlalchemy/event.py @@ -8,6 +8,7 @@ from . import util, exc from itertools import chain +import weakref CANCEL = util.symbol('CANCEL') NO_RETVAL = util.symbol('NO_RETVAL') @@ -242,11 +243,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), \ @@ -258,6 +260,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): @@ -271,9 +275,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: @@ -288,7 +296,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""" diff --git a/test/aaa_profiling/test_memusage.py b/test/aaa_profiling/test_memusage.py index 54c7d7ecc3..aabc0a2bc2 100644 --- a/test/aaa_profiling/test_memusage.py +++ b/test/aaa_profiling/test_memusage.py @@ -1,24 +1,23 @@ from sqlalchemy.testing import eq_ from sqlalchemy.orm import mapper, relationship, create_session, \ - clear_mappers, sessionmaker, class_mapper, aliased,\ + clear_mappers, sessionmaker, aliased,\ Session, subqueryload from sqlalchemy.orm.mapper import _mapper_registry from sqlalchemy.orm.session import _sessions -import operator from sqlalchemy import testing from sqlalchemy.testing import engines from sqlalchemy import MetaData, Integer, String, ForeignKey, \ - PickleType, create_engine, Unicode -from sqlalchemy.testing.schema import Table, Column + Unicode, select import sqlalchemy as sa +from sqlalchemy.testing.schema import Table, Column from sqlalchemy.sql import column from sqlalchemy.processors import to_decimal_processor_factory, \ to_unicode_processor_factory from sqlalchemy.testing.util import gc_collect from sqlalchemy.util.compat import decimal import gc -import weakref from sqlalchemy.testing import fixtures +import weakref class A(fixtures.ComparableEntity): pass @@ -32,13 +31,21 @@ def profile_memory(times=50): # 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, times)] for x in range(0, times): func(*args) gc_collect() - samples[x] = len(gc.get_objects()) + samples[x] = len(get_objects_skipping_sqlite_issue()) print "sample gc sizes:", samples @@ -145,6 +152,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) @@ -301,7 +320,7 @@ class MemUsageTest(EnsureZeroed): # pysqlite clearing out it's internal buffer and allow # the test to pass @testing.emits_warning() - @profile_memory(times=220) + @profile_memory() def go(): # execute with a non-unicode object. a warning is emitted, @@ -565,8 +584,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 @@ -585,8 +602,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, diff --git a/test/base/test_events.py b/test/base/test_events.py index eb58f51838..4efb30aba2 100644 --- a/test/base/test_events.py +++ b/test/base/test_events.py @@ -2,8 +2,9 @@ from sqlalchemy.testing import eq_, assert_raises, assert_raises_message, \ is_, is_not_ -from sqlalchemy import event, exc, util +from sqlalchemy import event, exc from sqlalchemy.testing import fixtures +from sqlalchemy.testing.util import gc_collect class EventsTest(fixtures.TestBase): """Test class- and instance-level event registration.""" @@ -359,6 +360,31 @@ class CustomTargetsTest(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 ListenOverrideTest(fixtures.TestBase): """Test custom listen functions which change the listener function signature.""" -- 2.47.2