Index: doc/CHANGES.rst =================================================================== --- doc/CHANGES.rst (revision 121792) +++ doc/CHANGES.rst (working copy) @@ -11,6 +11,11 @@ Bugs Fixed ++++++++++ +- Fix WSGIPublisher to close requests on abort as well. 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. + - Fix `WSGIResponse` and `publish_module` functions such that they support the `IStreamIterator` interface in addition to `file` (as supported by `ZServer.HTTPResponse`). Index: src/ZPublisher/tests/test_WSGIPublisher.py =================================================================== --- src/ZPublisher/tests/test_WSGIPublisher.py (revision 121792) +++ src/ZPublisher/tests/test_WSGIPublisher.py (working copy) @@ -414,6 +414,7 @@ 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 @@ _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): Index: src/ZPublisher/WSGIPublisher.py =================================================================== --- src/ZPublisher/WSGIPublisher.py (revision 121792) +++ src/ZPublisher/WSGIPublisher.py (working copy) @@ -200,6 +200,31 @@ 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 @@ 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 @@ # 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()