[CalendarServer-changes] [13696] twext/trunk/twext/enterprise

[source_changes at macosforge.org]

Revision: 13696
          http://trac.calendarserver.org//changeset/13696
Author:   cdaboo at apple.com
Date:     2014-06-27 12:22:06 -0700 (Fri, 27 Jun 2014)
Log Message:
-----------
Fix service termination race condition that would result in a terminated transaction continuing to sending SQL to the server.

Modified Paths:
--------------
    twext/trunk/twext/enterprise/adbapi2.py
    twext/trunk/twext/enterprise/fixtures.py
    twext/trunk/twext/enterprise/test/test_adbapi2.py

Modified: twext/trunk/twext/enterprise/adbapi2.py
===================================================================
--- twext/trunk/twext/enterprise/adbapi2.py	2014-06-27 19:20:13 UTC (rev 13695)
+++ twext/trunk/twext/enterprise/adbapi2.py	2014-06-27 19:22:06 UTC (rev 13696)
@@ -306,6 +306,8 @@
 
 
     def execSQL(self, *args, **kw):
+        if self._completed:
+            raise RuntimeError("Attempt to use {} transaction.".format(self._completed))
         result = self._holder.submit(
             lambda: self._reallyExecSQL(*args, **kw)
         )
@@ -322,12 +324,14 @@
         return result
 
 
-    def _end(self, really):
+    def _end(self, really, terminate=False):
         """
         Common logic for commit or abort.  Executed in the main reactor thread.
 
         @param really: the callable to execute in the cursor thread to actually
             do the commit or rollback.
+        @param terminate: abort and prevent any new SQL from being executed even
+            after a reset().
 
         @return: a L{Deferred} which fires when the database logic has
             completed.
@@ -336,7 +340,7 @@
             committed or aborted.
         """
         if not self._completed:
-            self._completed = "ended"
+            self._completed = "terminated" if terminate else "ended"
 
             def reallySomething():
                 """
@@ -346,7 +350,6 @@
                 if self._cursor is None or self._first:
                     return
                 really()
-                self._first = True
 
             result = self._holder.submit(reallySomething)
             self._pool._repoolAfter(self, result)
@@ -363,6 +366,10 @@
         return self._end(self._connection.rollback).addErrback(log.err)
 
 
+    def terminate(self):
+        return self._end(self._connection.rollback, terminate=True).addErrback(log.err)
+
+
     def reset(self):
         """
         Call this when placing this transaction back into the pool.
@@ -372,7 +379,9 @@
         """
         if not self._completed:
             raise RuntimeError("Attempt to re-set active transaction.")
-        self._completed = False
+        if self._completed != "terminated":
+            self._completed = False
+        self._first = True
 
 
     def _releaseConnection(self):
@@ -919,7 +928,10 @@
         return d
 
 
+    def terminate(self):
+        return self.abort()
 
+
 def _fork(x):
     """
     Produce a L{Deferred} that will fire when another L{Deferred} fires without
@@ -1034,10 +1046,11 @@
         while self._finishing:
             yield _fork(self._finishing[0][1])
 
-        # Phase 3: All of the busy transactions must be aborted first.  As each
-        # one is aborted, it will remove itself from the list.
+        # Phase 3: All of the busy transactions must be terminated first.  As each
+        # one is terminated, it will remove itself from the list. Note we terminate
+        # and not abort the transaction to ensure they cannot be re-used.
         while self._busy:
-            yield self._busy[0].abort()
+            yield self._busy[0].terminate()
 
         # Phase 4: All transactions should now be in the free list, since
         # "abort()" will have put them there.  Shut down all the associated

Modified: twext/trunk/twext/enterprise/fixtures.py
===================================================================
--- twext/trunk/twext/enterprise/fixtures.py	2014-06-27 19:20:13 UTC (rev 13695)
+++ twext/trunk/twext/enterprise/fixtures.py	2014-06-27 19:22:06 UTC (rev 13696)
@@ -458,6 +458,8 @@
 
 
     def execute(self, sql, args=()):
+        if self.connection.closed:
+            raise FakeConnectionError
         self.connection.executions += 1
         if self.connection._executeFailQueue:
             raise self.connection._executeFailQueue.pop(0)()

Modified: twext/trunk/twext/enterprise/test/test_adbapi2.py
===================================================================
--- twext/trunk/twext/enterprise/test/test_adbapi2.py	2014-06-27 19:20:13 UTC (rev 13695)
+++ twext/trunk/twext/enterprise/test/test_adbapi2.py	2014-06-27 19:22:06 UTC (rev 13696)
@@ -463,6 +463,25 @@
         self.assertEquals(self.factory.connections[1].closed, True)
 
 
+    def test_partialTxnFailsDuringStopService(self):
+        """
+        Using the logic in L{ConnectionPool.stopService}, make sure that an
+        L{_ConnectedTxn} cannot continue to process SQL after L{_ConnectedTxn.abort}
+        is called and before L{_ConnectedTxn.reset} is called.
+        """
+        txn = self.createTransaction()
+        if hasattr(txn, "_baseTxn"):
+            # Send initial statement
+            txn.execSQL("maybe change something!")
+
+            # Make it look like the service is stopping
+            txn._baseTxn._connection.close()
+            txn._baseTxn.terminate()
+
+            # Try to send more SQL - must fail
+            self.failUnlessRaises(RuntimeError, txn.execSQL, "maybe change something else!")
+
+
     def test_abortRecycledTransaction(self):
         """
         L{ConnectionPool.stopService} will shut down if a recycled transaction
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.macosforge.org/pipermail/calendarserver-changes/attachments/20140627/33ae70df/attachment-0001.html>