Merge pull request #19 from zopefoundation/16-fix_leak_w_broken_EndRequestEvent_subscriber-master

Prevent leaked connections when broken 'EndRequestEvent' subscribers raise

CHANGES.rst

@@ -11,6 +11,9 @@ http://docs.zope.org/zope2/
 Bugs Fixed
 ++++++++++
 
+- Issue #16: prevent leaked connections when broken ``EndRequestEvent``
+  subscribers raise exceptions.
+
 - Ensure that the ``WSGIPublisher`` begins and ends an *interaction*
   at the request/response barrier. This is required for instance for
   the ``checkPermission`` call to function without an explicit

src/ZPublisher/BaseRequest.py

@@ -215,10 +215,12 @@ class BaseRequest:
         self._held=None
 
     def close(self):
-        notify(EndRequestEvent(None, self))
-        # subscribers might need the zodb, so `clear` must come afterwards
-        # (since `self._held=None` might close the connection, see above)
-        self.clear()
+        try:
+            notify(EndRequestEvent(None, self))
+        finally:
+            # subscribers might need the zodb, so `clear` must come afterwards
+            # (since `self._held=None` might close the connection, see above)
+            self.clear()
 
     def processInputs(self):
         """Do any input processing that could raise errors

src/ZPublisher/tests/testBaseRequest.py

@@ -424,6 +424,22 @@ class TestBaseRequest(unittest.TestCase, BaseRequest_factory):
         r = self._makeOne(root)
         self.assertRaises(NotFound, r.traverse, 'folder/simpleFrozenSet')
 
+    def test_close_w_broken_subscriber(self):
+        # See: https://github.com/zopefoundation/Zope/issues/16
+        from zope.event import subscribers
+        root, folder = self._makeRootAndFolder()
+        r = self._makeOne(root)
+        r.other['foo'] = 'Foo'
+        BEFORE = subscribers[:]
+        def _broken(event):
+            raise ValueError("I'm broken")
+        subscribers.append(_broken)
+        try:
+            self.assertRaises(ValueError, r.close)
+        finally:
+            subscribers[:] = BEFORE
+        self.assertEqual(r.other, {})
+
     def test_hold_after_close(self):
         # Request should no longer accept holds after it has been closed
         root, folder = self._makeRootAndFolder()