Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #19707 -- Reset transaction state after requests

  • Loading branch information...
commit a4e97cf315142e61bb4bc3ed8259b95d8586d09c 1 parent 0e18fb0
Anssi Kääriäinen authored February 05, 2013
13  django/db/__init__.py
@@ -42,8 +42,17 @@ def __setattr__(self, name, value):
42 42
 # Register an event that closes the database connection
43 43
 # when a Django request is finished.
44 44
 def close_connection(**kwargs):
45  
-    for conn in connections.all():
46  
-        conn.close()
  45
+    # Avoid circular imports
  46
+    from django.db import transaction
  47
+    for conn in connections:
  48
+        try:
  49
+            transaction.abort(conn)
  50
+            connections[conn].close()
  51
+        except Exception:
  52
+            # The connection's state is unknown, so it has to be
  53
+            # abandoned. This could happen for example if the network
  54
+            # connection has a failure.
  55
+            del connections[conn]
47 56
 signals.request_finished.connect(close_connection)
48 57
 
49 58
 # Register an event that resets connection.queries
11  django/db/backends/__init__.py
@@ -88,6 +88,17 @@ def _savepoint_commit(self, sid):
88 88
             return
89 89
         self.cursor().execute(self.ops.savepoint_commit_sql(sid))
90 90
 
  91
+    def abort(self):
  92
+        """
  93
+        Roll back any ongoing transaction and clean the transaction state
  94
+        stack.
  95
+        """
  96
+        if self._dirty:
  97
+            self._rollback()
  98
+            self._dirty = False
  99
+        while self.transaction_state:
  100
+            self.leave_transaction_management()
  101
+
91 102
     def enter_transaction_management(self, managed=True):
92 103
         """
93 104
         Enters transaction management for a running thread. It must be balanced with
15  django/db/transaction.py
@@ -24,6 +24,21 @@ class TransactionManagementError(Exception):
24 24
     """
25 25
     pass
26 26
 
  27
+def abort(using=None):
  28
+    """
  29
+    Roll back any ongoing transactions and clean the transaction management
  30
+    state of the connection.
  31
+
  32
+    This method is to be used only in cases where using balanced
  33
+    leave_transaction_management() calls isn't possible. For example after a
  34
+    request has finished, the transaction state isn't known, yet the connection
  35
+    must be cleaned up for the next request.
  36
+    """
  37
+    if using is None:
  38
+        using = DEFAULT_DB_ALIAS
  39
+    connection = connections[using]
  40
+    connection.abort()
  41
+
27 42
 def enter_transaction_management(managed=True, using=None):
28 43
     """
29 44
     Enters transaction management for a running thread. It must be balanced with
3  django/db/utils.py
@@ -99,6 +99,9 @@ def __getitem__(self, alias):
99 99
     def __setitem__(self, key, value):
100 100
         setattr(self._connections, key, value)
101 101
 
  102
+    def __delitem__(self, key):
  103
+        delattr(self._connections, key)
  104
+
102 105
     def __iter__(self):
103 106
         return iter(self.databases)
104 107
 
21  django/middleware/transaction.py
@@ -15,6 +15,10 @@ def process_request(self, request):
15 15
     def process_exception(self, request, exception):
16 16
         """Rolls back the database and leaves transaction management"""
17 17
         if transaction.is_dirty():
  18
+            # This rollback might fail because of network failure for example.
  19
+            # If rollback isn't possible it is impossible to clean the
  20
+            # connection's state. So leave the connection in dirty state and
  21
+            # let request_finished signal deal with cleaning the connection.
18 22
             transaction.rollback()
19 23
         transaction.leave_transaction_management()
20 24
 
@@ -22,6 +26,21 @@ def process_response(self, request, response):
22 26
         """Commits and leaves transaction management."""
23 27
         if transaction.is_managed():
24 28
             if transaction.is_dirty():
25  
-                transaction.commit()
  29
+                # Note: it is possible that the commit fails. If the reason is
  30
+                # closed connection or some similar reason, then there is
  31
+                # little hope to proceed nicely. However, in some cases (
  32
+                # deferred foreign key checks for exampl) it is still possible
  33
+                # to rollback().
  34
+                try:
  35
+                    transaction.commit()
  36
+                except Exception:
  37
+                    # If the rollback fails, the transaction state will be
  38
+                    # messed up. It doesn't matter, the connection will be set
  39
+                    # to clean state after the request finishes. And, we can't
  40
+                    # clean the state here properly even if we wanted to, the
  41
+                    # connection is in transaction but we can't rollback...
  42
+                    transaction.rollback()
  43
+                    transaction.leave_transaction_management()
  44
+                    raise
26 45
             transaction.leave_transaction_management()
27 46
         return response
3  django/test/testcases.py
@@ -70,6 +70,7 @@ def to_list(value):
70 70
 real_enter_transaction_management = transaction.enter_transaction_management
71 71
 real_leave_transaction_management = transaction.leave_transaction_management
72 72
 real_managed = transaction.managed
  73
+real_abort = transaction.abort
73 74
 
74 75
 def nop(*args, **kwargs):
75 76
     return
@@ -80,6 +81,7 @@ def disable_transaction_methods():
80 81
     transaction.enter_transaction_management = nop
81 82
     transaction.leave_transaction_management = nop
82 83
     transaction.managed = nop
  84
+    transaction.abort = nop
83 85
 
84 86
 def restore_transaction_methods():
85 87
     transaction.commit = real_commit
@@ -87,6 +89,7 @@ def restore_transaction_methods():
87 89
     transaction.enter_transaction_management = real_enter_transaction_management
88 90
     transaction.leave_transaction_management = real_leave_transaction_management
89 91
     transaction.managed = real_managed
  92
+    transaction.abort = real_abort
90 93
 
91 94
 
92 95
 def assert_and_parse_html(self, html, user_msg, msg):
25  tests/regressiontests/middleware/tests.py
@@ -9,9 +9,9 @@
9 9
 
10 10
 from django.conf import settings
11 11
 from django.core import mail
12  
-from django.db import transaction
13  
-from django.http import HttpRequest
14  
-from django.http import HttpResponse, StreamingHttpResponse
  12
+from django.db import (transaction, connections, DEFAULT_DB_ALIAS,
  13
+                       IntegrityError)
  14
+from django.http import HttpRequest, HttpResponse, StreamingHttpResponse
15 15
 from django.middleware.clickjacking import XFrameOptionsMiddleware
16 16
 from django.middleware.common import CommonMiddleware, BrokenLinkEmailsMiddleware
17 17
 from django.middleware.http import ConditionalGetMiddleware
@@ -710,3 +710,22 @@ def test_exception(self):
710 710
         TransactionMiddleware().process_exception(self.request, None)
711 711
         self.assertEqual(Band.objects.count(), 0)
712 712
         self.assertFalse(transaction.is_dirty())
  713
+
  714
+    def test_failing_commit(self):
  715
+        # It is possible that connection.commit() fails. Check that
  716
+        # TransactionMiddleware handles such cases correctly.
  717
+        try:
  718
+            def raise_exception():
  719
+                raise IntegrityError()
  720
+            connections[DEFAULT_DB_ALIAS].commit = raise_exception
  721
+            transaction.enter_transaction_management()
  722
+            transaction.managed(True)
  723
+            Band.objects.create(name='The Beatles')
  724
+            self.assertTrue(transaction.is_dirty())
  725
+            with self.assertRaises(IntegrityError):
  726
+                TransactionMiddleware().process_response(self.request, None)
  727
+            self.assertEqual(Band.objects.count(), 0)
  728
+            self.assertFalse(transaction.is_dirty())
  729
+            self.assertFalse(transaction.is_managed())
  730
+        finally:
  731
+            del connections[DEFAULT_DB_ALIAS].commit
42  tests/regressiontests/requests/tests.py
@@ -6,9 +6,12 @@
6 6
 from datetime import datetime, timedelta
7 7
 from io import BytesIO
8 8
 
  9
+from django.db import connection, connections, DEFAULT_DB_ALIAS
  10
+from django.core import signals
9 11
 from django.core.exceptions import SuspiciousOperation
10 12
 from django.core.handlers.wsgi import WSGIRequest, LimitedStream
11 13
 from django.http import HttpRequest, HttpResponse, parse_cookie, build_request_repr, UnreadablePostError
  14
+from django.test import TransactionTestCase
12 15
 from django.test.client import FakePayload
13 16
 from django.test.utils import override_settings, str_prefix
14 17
 from django.utils import six
@@ -524,3 +527,42 @@ def read(self, len=0):
524 527
 
525 528
         with self.assertRaises(UnreadablePostError):
526 529
             request.body
  530
+
  531
+class TransactionRequestTests(TransactionTestCase):
  532
+    def test_request_finished_db_state(self):
  533
+        # The GET below will not succeed, but it will give a response with
  534
+        # defined ._handler_class. That is needed for sending the
  535
+        # request_finished signal.
  536
+        response = self.client.get('/')
  537
+        # Make sure there is an open connection
  538
+        connection.cursor()
  539
+        connection.enter_transaction_management()
  540
+        connection.managed(True)
  541
+        signals.request_finished.send(sender=response._handler_class)
  542
+        # In-memory sqlite doesn't actually close connections.
  543
+        if connection.vendor != 'sqlite':
  544
+            self.assertIs(connection.connection, None)
  545
+        self.assertEqual(len(connection.transaction_state), 0)
  546
+
  547
+    @unittest.skipIf(connection.vendor == 'sqlite',
  548
+                     'This test will close the connection, in-memory '
  549
+                     'sqlite connections must not be closed.')
  550
+    def test_request_finished_failed_connection(self):
  551
+        # See comments in test_request_finished_db_state() for the self.client
  552
+        # usage.
  553
+        response = self.client.get('/')
  554
+        conn = connections[DEFAULT_DB_ALIAS]
  555
+        conn.enter_transaction_management()
  556
+        conn.managed(True)
  557
+        conn.set_dirty()
  558
+        # Test that the rollback doesn't succeed (for example network failure
  559
+        # could cause this).
  560
+        def fail_horribly():
  561
+            raise Exception("Horrible failure!")
  562
+        conn._rollback = fail_horribly
  563
+        signals.request_finished.send(sender=response._handler_class)
  564
+        # As even rollback wasn't possible the connection wrapper itself was
  565
+        # abandoned. Accessing the connections[alias] will create a new
  566
+        # connection wrapper, whch must be different than the original one.
  567
+        self.assertIsNot(conn, connections[DEFAULT_DB_ALIAS])
  568
+        self.assertEqual(len(connection.transaction_state), 0)

0 notes on commit a4e97cf

Please sign in to comment.
Something went wrong with that request. Please try again.