Merge r121832 from trunk to fix LP: #787541

doc/CHANGES.rst

@@ -11,6 +11,10 @@ http://docs.zope.org/zope2/releases/.
 Bugs Fixed
 ++++++++++
 
+- LP #787541: Fix WSGIPublisher to close requests on abort unconditionally.
+  Previously an addAfterCommitHook was used, but this is not run on transaction
+  aborts.  Now a Synchronizer is used which unconditionally closes the request
+  after a transaction is finished.
 
 Features Added
 ++++++++++++++

src/ZPublisher/WSGIPublisher.py

@@ -200,6 +200,31 @@ def publish(request, module_name,
 
     return response
 
+class _RequestCloserForTransaction(object):
+    """Unconditionally close the request at the end of a transaction.
+    
+    See transaction.interfaces.ISynchronizer
+    """
+
+    def __init__(self):
+        self.requests = {}
+
+    def add(self, txn, request):
+        assert txn not in self.requests
+        self.requests[txn] = request
+
+    def beforeCompletion(self, txn):
+        pass
+
+    newTransaction = beforeCompletion
+
+    def afterCompletion(self, txn):
+        request = self.requests.pop(txn, None)
+        if request is not None:
+            request.close()
+
+_request_closer_for_repoze_tm = _RequestCloserForTransaction()
+
 def publish_module(environ, start_response,
                    _publish=publish,                # only for testing
                    _response_factory=WSGIResponse,  # only for testing
@@ -214,6 +239,13 @@ def publish_module(environ, start_response,
     response._server_version = environ.get('SERVER_SOFTWARE')
 
     request = _request_factory(environ['wsgi.input'], environ, response)
+
+    if 'repoze.tm.active' in environ:
+        # NOTE: registerSynch is a no-op after the first request
+        transaction.manager.registerSynch(_request_closer_for_repoze_tm)
+        txn = transaction.get()
+        _request_closer_for_repoze_tm.add(txn, request)
+
     setDefaultSkin(request)
 
     try:
@@ -237,10 +269,7 @@ def publish_module(environ, start_response,
         # XXX This still needs verification that it really works.
         result = (stdout.getvalue(), response.body)
 
-    if 'repoze.tm.active' in environ:
-        txn = transaction.get()
-        txn.addAfterCommitHook(lambda ok: request.close())
-    else:
+    if 'repoze.tm.active' not in environ:
         request.close() # this aborts the transation!
 
     stdout.close()

src/ZPublisher/tests/test_WSGIPublisher.py

@@ -414,6 +414,7 @@ class Test_publish_module(unittest.TestCase):
 
     def test_request_not_closed_when_tm_middleware_active(self):
         import transaction
+        from ZPublisher import WSGIPublisher
         environ = self._makeEnviron()
         environ['repoze.tm.active'] = 1
         start_response = DummyCallable()
@@ -430,7 +431,22 @@ class Test_publish_module(unittest.TestCase):
                                  _request_factory=_request_factory)
         self.assertFalse(_request._closed)
         txn = transaction.get()
-        self.assertTrue(list(txn.getAfterCommitHooks()))
+        self.assertTrue(txn in WSGIPublisher._request_closer_for_repoze_tm.requests)
+        txn.commit()
+        self.assertTrue(_request._closed)
+        self.assertFalse(txn in WSGIPublisher._request_closer_for_repoze_tm.requests)
+        # try again, but this time raise an exception and abort
+        _request._closed = False
+        _publish._raise = Exception('oops')
+        self.assertRaises(Exception, self._callFUT,
+                          environ, start_response, _publish,
+                          _request_factory=_request_factory)
+        self.assertFalse(_request._closed)
+        txn = transaction.get()
+        self.assertTrue(txn in WSGIPublisher._request_closer_for_repoze_tm.requests)
+        txn.abort()
+        self.assertFalse(txn in WSGIPublisher._request_closer_for_repoze_tm.requests)
+        self.assertTrue(_request._closed)
 
 
 class DummyRequest(dict):