diff --git a/CHANGES.rst b/CHANGES.rst index d2dbe071b4..a8e4d6c61a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,9 @@ +Unreleased +========== + +- Break potential reference cycle between ``request`` and ``context``. + See https://github.com/Pylons/pyramid/pull/3649 + 2.0b0 (2020-12-15) ================== diff --git a/src/pyramid/router.py b/src/pyramid/router.py index 644a6a3953..61660c41be 100644 --- a/src/pyramid/router.py +++ b/src/pyramid/router.py @@ -252,8 +252,12 @@ def invoke_request(self, request, _use_tweens=True): return response finally: - if request.finished_callbacks: - request._process_finished_callbacks() + self.finish_request(request) + + def finish_request(self, request): + if request.finished_callbacks: + request._process_finished_callbacks() + request.__dict__.pop('context', None) # Break potential ref cycle def __call__(self, environ, start_response): """ diff --git a/tests/test_router.py b/tests/test_router.py index 0f00ea531a..857e8aea5d 100644 --- a/tests/test_router.py +++ b/tests/test_router.py @@ -124,6 +124,19 @@ def _makeOne(self): klass = self._getTargetClass() return klass(self.registry) + def _mockFinishRequest(self, router): + """ + Mock :meth:`pyramid.router.Router.finish_request` to be a no-op. This + prevents :prop:`pyramid.request.Request.context` from being removed, so + we can write assertions against it. + + """ + + def mock_finish_request(request): + pass + + router.finish_request = mock_finish_request + def _makeEnviron(self, **extras): environ = { 'wsgi.url_scheme': 'http', @@ -421,6 +434,7 @@ def test_call_view_registered_nonspecific_default_path(self): ) self._registerRootFactory(context) router = self._makeOne() + self._mockFinishRequest(router) start_response = DummyStartResponse() result = router(environ, start_response) self.assertEqual(result, ['Hello world']) @@ -448,6 +462,7 @@ def test_call_view_registered_nonspecific_nondefault_path_and_subpath( environ = self._makeEnviron() self._registerView(view, 'foo', IViewClassifier, None, None) router = self._makeOne() + self._mockFinishRequest(router) start_response = DummyStartResponse() result = router(environ, start_response) self.assertEqual(result, ['Hello world']) @@ -477,6 +492,7 @@ class IContext(Interface): environ = self._makeEnviron() self._registerView(view, '', IViewClassifier, IRequest, IContext) router = self._makeOne() + self._mockFinishRequest(router) start_response = DummyStartResponse() result = router(environ, start_response) self.assertEqual(result, ['Hello world']) @@ -618,7 +634,7 @@ def callback(request, response): router(environ, start_response) self.assertEqual(response.called_back, True) - def test_call_request_has_finished_callbacks_when_view_succeeds(self): + def test_finish_request_when_view_succeeds(self): from zope.interface import Interface, directlyProvides class IContext(Interface): @@ -636,6 +652,7 @@ def callback(request): request.environ['called_back'] = True request.add_finished_callback(callback) + request.environ['request'] = request return response environ = self._makeEnviron() @@ -644,8 +661,9 @@ def callback(request): start_response = DummyStartResponse() router(environ, start_response) self.assertEqual(environ['called_back'], True) + self.assertFalse(hasattr(environ['request'], 'context')) - def test_call_request_has_finished_callbacks_when_view_raises(self): + def test_finish_request_when_view_raises(self): from zope.interface import Interface, directlyProvides class IContext(Interface): @@ -662,6 +680,7 @@ def callback(request): request.environ['called_back'] = True request.add_finished_callback(callback) + request.environ['request'] = request raise NotImplementedError environ = self._makeEnviron() @@ -670,6 +689,7 @@ def callback(request): start_response = DummyStartResponse() exc_raised(NotImplementedError, router, environ, start_response) self.assertEqual(environ['called_back'], True) + self.assertFalse(hasattr(environ['request'], 'context')) def test_call_request_factory_raises(self): # making sure finally doesnt barf when a request cannot be created @@ -704,6 +724,7 @@ def test_call_eventsends(self): context_found_events = self._registerEventListener(IContextFound) response_events = self._registerEventListener(INewResponse) router = self._makeOne() + self._mockFinishRequest(router) start_response = DummyStartResponse() result = router(environ, start_response) self.assertEqual(len(request_events), 1) @@ -767,6 +788,7 @@ def factory(request): self._registerView(view, '', IViewClassifier, None, None) self._registerRootFactory(context) router = self._makeOne() + self._mockFinishRequest(router) start_response = DummyStartResponse() result = router(environ, start_response) self.assertEqual(result, ['Hello world']) @@ -841,6 +863,7 @@ def factory(request): self._registerView(view, '', IViewClassifier, None, None) self._registerRootFactory(context) router = self._makeOne() + self._mockFinishRequest(router) start_response = DummyStartResponse() result = router(environ, start_response) self.assertEqual(result, ['Hello world'])