Merge pull request #3649 from luhn/context-break-cycles

Break reference cycle between request and context.

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)
 ==================
 

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):
         """

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'])