SVN: Zope/trunk/ LP #787541: Fix WSGIPublisher to close requests on abort unconditionally.

Log message for revision 121832:
  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.
  
  Not closing the request seems to have caused the database connections to be
  kept open and these nasty log messages:
  
      CRITICAL ZODB.DB DB.open() has 42 open connections with a pool_size of 7
  
  42 being the wrong answer in this case.
  

Changed:
  U   Zope/trunk/doc/CHANGES.rst
  U   Zope/trunk/src/ZPublisher/WSGIPublisher.py
  U   Zope/trunk/src/ZPublisher/tests/test_WSGIPublisher.py

-=-
Modified: Zope/trunk/doc/CHANGES.rst
===================================================================
--- Zope/trunk/doc/CHANGES.rst	2011-05-30 10:07:08 UTC (rev 121831)
+++ Zope/trunk/doc/CHANGES.rst	2011-05-30 11:47:48 UTC (rev 121832)
@@ -11,6 +11,11 @@
 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.
+
 - Fix `WSGIResponse` and `publish_module` functions such that they
   support the `IStreamIterator` interface in addition to `file` (as
   supported by `ZServer.HTTPResponse`).

Modified: Zope/trunk/src/ZPublisher/WSGIPublisher.py
===================================================================
--- Zope/trunk/src/ZPublisher/WSGIPublisher.py	2011-05-30 10:07:08 UTC (rev 121831)
+++ Zope/trunk/src/ZPublisher/WSGIPublisher.py	2011-05-30 11:47:48 UTC (rev 121832)
@@ -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()

Modified: Zope/trunk/src/ZPublisher/tests/test_WSGIPublisher.py
===================================================================
--- Zope/trunk/src/ZPublisher/tests/test_WSGIPublisher.py	2011-05-30 10:07:08 UTC (rev 121831)
+++ Zope/trunk/src/ZPublisher/tests/test_WSGIPublisher.py	2011-05-30 11:47:48 UTC (rev 121832)
@@ -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):

_______________________________________________
Zope-Checkins maillist  -  [email protected]
https://mail.zope.org/mailman/listinfo/zope-checkins