From 0393f96a8ef8a9336f35816f9be6898e99014b94 Mon Sep 17 00:00:00 2001 From: Kirill Kuzminykh Date: Wed, 1 Mar 2017 11:52:25 +0300 Subject: [PATCH 1/5] Fixed several reference cycles to prevent memory leaks. Added simple test for detect memory leaks after application closing. --- pyramid/config/views.py | 2 +- pyramid/tests/test_config/test_views.py | 4 +++- pyramid/tests/test_integration.py | 26 +++++++++++++++++++++++++ pyramid/tests/test_tweens.py | 4 ++++ pyramid/tweens.py | 2 +- pyramid/util.py | 9 ++++++--- 6 files changed, 41 insertions(+), 6 deletions(-) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 65c9da5855..b6996b6d26 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -933,7 +933,7 @@ def register(permission=permission, renderer=renderer): if not exception_only and isexc: derived_view = runtime_exc_view(derived_view, derived_exc_view) - derived_view.__discriminator__ = lambda *arg: discriminator + derived_view.__discriminator__ = lambda *arg: view_intr.discriminator # __discriminator__ is used by superdynamic systems # that require it for introspection after manual view lookup; # see also MultiView.__discriminator__ diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 211632730a..79631c6b80 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -13,6 +13,8 @@ from pyramid.exceptions import ConfigurationError from pyramid.exceptions import ConfigurationExecutionError from pyramid.exceptions import ConfigurationConflictError +from pyramid.registry import undefer + class TestViewsConfigurationMixin(unittest.TestCase): def _makeOne(self, *arg, **kw): @@ -148,7 +150,7 @@ def view(request): # request-only wrapper self.assertEqual(wrapper.__module__, view.__module__) self.assertEqual(wrapper.__name__, view.__name__) self.assertEqual(wrapper.__doc__, view.__doc__) - self.assertEqual(wrapper.__discriminator__(None, None).resolve()[0], + self.assertEqual(undefer(wrapper.__discriminator__(None, None))[0], 'view') def test_add_view_view_callable_dottedname(self): diff --git a/pyramid/tests/test_integration.py b/pyramid/tests/test_integration.py index 85c4466a42..09ea85e13c 100644 --- a/pyramid/tests/test_integration.py +++ b/pyramid/tests/test_integration.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import datetime +import gc import locale import os import unittest @@ -741,3 +742,28 @@ def _assertBody(body, filename): data = data.replace(b'\r', b'') data = data.replace(b'\n', b'') assert(body == data) + + +class MemoryLeaksTest(unittest.TestCase): + + def tearDown(self): + import pyramid.config + pyramid.config.global_registries.empty() + + def get_gc_count(self): + last_collected = 0 + while True: + collected = gc.collect() + if collected == last_collected: + break + last_collected = collected + return len(gc.get_objects()) + + def test_memory_leaks(self): + from pyramid.config import Configurator + Configurator().make_wsgi_app() # Initialize all global objects + + initial_count = self.get_gc_count() + Configurator().make_wsgi_app() + current_count = self.get_gc_count() + self.assertEqual(current_count, initial_count) diff --git a/pyramid/tests/test_tweens.py b/pyramid/tests/test_tweens.py index c8eada34cf..d2053e6b88 100644 --- a/pyramid/tests/test_tweens.py +++ b/pyramid/tests/test_tweens.py @@ -31,6 +31,7 @@ def handler(request): raise HTTPNotFound tween = self._makeOne(handler) request = Request.blank('/') + request.registry = self.config.registry result = tween(request) self.assertEqual(result.status, '404 Not Found') @@ -44,6 +45,7 @@ def handler(request): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') + request.registry = self.config.registry result = tween(request) self.assertTrue(b'foo' in result.body) @@ -55,6 +57,7 @@ def handler(request): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') + request.registry = self.config.registry request.method = 'POST' self.assertRaises(ValueError, lambda: tween(request)) @@ -64,6 +67,7 @@ def handler(request): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') + request.registry = self.config.registry self.assertRaises(ValueError, lambda: tween(request)) class DummyRequest: diff --git a/pyramid/tweens.py b/pyramid/tweens.py index a842b11333..19daf9e5e5 100644 --- a/pyramid/tweens.py +++ b/pyramid/tweens.py @@ -42,7 +42,7 @@ def excview_tween(request): provides = providedBy(exc) try: response = _call_view( - registry, + request.registry, request, exc, provides, diff --git a/pyramid/util.py b/pyramid/util.py index 3337d410db..2827884a34 100644 --- a/pyramid/util.py +++ b/pyramid/util.py @@ -231,17 +231,20 @@ def add(self, item): self._order.remove(oid) self._order.append(oid) return - ref = weakref.ref(item, lambda x: self.remove(item)) + ref = weakref.ref(item, lambda x: self._remove_by_id(oid)) self._items[oid] = ref self._order.append(oid) - def remove(self, item): + def _remove_by_id(self, oid): """ Remove an item from the set.""" - oid = id(item) if oid in self._items: del self._items[oid] self._order.remove(oid) + def remove(self, item): + """ Remove an item from the set.""" + self._remove_by_id(id(item)) + def empty(self): """ Clear all objects from the set.""" self._items = {} From 5f012c616d8b593c3bf310e8115563f4e4c04971 Mon Sep 17 00:00:00 2001 From: Kirill Kuzminykh Date: Wed, 1 Mar 2017 12:00:21 +0300 Subject: [PATCH 2/5] Added a new contributor into the CONTRIBUTORS.txt --- CONTRIBUTORS.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index d5c1784185..566e911952 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -292,3 +292,5 @@ Contributors - Mikko Ohtamaa, 2016/12/6 - Martin Frlin, 2016/12/7 + +- Kirill Kuzminykh, 2017/03/01 From b1cad5ee8c8bdf8dd5819e77366c36869d53edbb Mon Sep 17 00:00:00 2001 From: Kirill Kuzminykh Date: Wed, 1 Mar 2017 13:00:10 +0300 Subject: [PATCH 3/5] The memory leaks test skipped for platform 'pypy'. --- pyramid/tests/test_integration.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyramid/tests/test_integration.py b/pyramid/tests/test_integration.py index 09ea85e13c..f23e546091 100644 --- a/pyramid/tests/test_integration.py +++ b/pyramid/tests/test_integration.py @@ -9,6 +9,7 @@ from pyramid.wsgi import wsgiapp from pyramid.view import view_config from pyramid.static import static_view +from pyramid.testing import skip_on from pyramid.compat import ( text_, url_quote, @@ -759,6 +760,7 @@ def get_gc_count(self): last_collected = collected return len(gc.get_objects()) + @skip_on('pypy') def test_memory_leaks(self): from pyramid.config import Configurator Configurator().make_wsgi_app() # Initialize all global objects From 71e92df9249274796224ae6d72ca83f0bca942d7 Mon Sep 17 00:00:00 2001 From: Kirill Kuzminykh Date: Thu, 2 Mar 2017 22:40:36 +0300 Subject: [PATCH 4/5] Reverted couple useless fixes of memory leaks. --- pyramid/config/views.py | 2 +- pyramid/registry.py | 4 +++- pyramid/tests/test_tweens.py | 4 ---- pyramid/tweens.py | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index b6996b6d26..65c9da5855 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -933,7 +933,7 @@ def register(permission=permission, renderer=renderer): if not exception_only and isexc: derived_view = runtime_exc_view(derived_view, derived_exc_view) - derived_view.__discriminator__ = lambda *arg: view_intr.discriminator + derived_view.__discriminator__ = lambda *arg: discriminator # __discriminator__ is used by superdynamic systems # that require it for introspection after manual view lookup; # see also MultiView.__discriminator__ diff --git a/pyramid/registry.py b/pyramid/registry.py index 20b3643e92..7589dfcac7 100644 --- a/pyramid/registry.py +++ b/pyramid/registry.py @@ -276,7 +276,9 @@ def __init__(self, func): @reify def value(self): - return self.func() + result = self.func() + del self.func + return result def resolve(self): return self.value diff --git a/pyramid/tests/test_tweens.py b/pyramid/tests/test_tweens.py index d2053e6b88..c8eada34cf 100644 --- a/pyramid/tests/test_tweens.py +++ b/pyramid/tests/test_tweens.py @@ -31,7 +31,6 @@ def handler(request): raise HTTPNotFound tween = self._makeOne(handler) request = Request.blank('/') - request.registry = self.config.registry result = tween(request) self.assertEqual(result.status, '404 Not Found') @@ -45,7 +44,6 @@ def handler(request): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') - request.registry = self.config.registry result = tween(request) self.assertTrue(b'foo' in result.body) @@ -57,7 +55,6 @@ def handler(request): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') - request.registry = self.config.registry request.method = 'POST' self.assertRaises(ValueError, lambda: tween(request)) @@ -67,7 +64,6 @@ def handler(request): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') - request.registry = self.config.registry self.assertRaises(ValueError, lambda: tween(request)) class DummyRequest: diff --git a/pyramid/tweens.py b/pyramid/tweens.py index 19daf9e5e5..a842b11333 100644 --- a/pyramid/tweens.py +++ b/pyramid/tweens.py @@ -42,7 +42,7 @@ def excview_tween(request): provides = providedBy(exc) try: response = _call_view( - request.registry, + registry, request, exc, provides, From 870a1d1f957d772cab0eb60bc1d1928da2ebb992 Mon Sep 17 00:00:00 2001 From: Kirill Kuzminykh Date: Thu, 2 Mar 2017 22:43:05 +0300 Subject: [PATCH 5/5] Reverted useless changes in tests. --- pyramid/tests/test_config/test_views.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 79631c6b80..211632730a 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -13,8 +13,6 @@ from pyramid.exceptions import ConfigurationError from pyramid.exceptions import ConfigurationExecutionError from pyramid.exceptions import ConfigurationConflictError -from pyramid.registry import undefer - class TestViewsConfigurationMixin(unittest.TestCase): def _makeOne(self, *arg, **kw): @@ -150,7 +148,7 @@ def view(request): # request-only wrapper self.assertEqual(wrapper.__module__, view.__module__) self.assertEqual(wrapper.__name__, view.__name__) self.assertEqual(wrapper.__doc__, view.__doc__) - self.assertEqual(undefer(wrapper.__discriminator__(None, None))[0], + self.assertEqual(wrapper.__discriminator__(None, None).resolve()[0], 'view') def test_add_view_view_callable_dottedname(self):