Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #21202 -- Maintained atomicity when the server disconnects.

Thanks intgr for the report.

This commit doesn't include a test because I don't know how to emulate a
database disconnection in a cross-database compatible way.

Also simplified a 'backends' test that was constrained by this problem.
  • Loading branch information...
commit 8176150850b2e34b2afe1dc107e184eb4c6cd668 1 parent 2586009
@aaugustin aaugustin authored
Showing with 13 additions and 10 deletions.
  1. +13 −3 django/db/transaction.py
  2. +0 −7 tests/backends/tests.py
View
16 django/db/transaction.py
@@ -2,7 +2,7 @@
from django.db import (
connections, DEFAULT_DB_ALIAS,
- DatabaseError, ProgrammingError)
+ DatabaseError, Error, ProgrammingError)
from django.utils.decorators import available_attrs
@@ -224,7 +224,12 @@ def __exit__(self, exc_type, exc_value, traceback):
try:
connection.commit()
except DatabaseError:
- connection.rollback()
+ try:
+ connection.rollback()
+ except Error:
+ # Error during rollback means the connection was
+ # closed. Clean up in case the server dropped it.
+ connection.close()
@intgr
intgr added a note

I think this comment might be slightly misleading, an error from rollback doesn't always mean a closed connection. But the coding seems prudent: we no longer know what state the transaction is in, so it's best to kill it. Much better than my patch.

@aaugustin Owner

Fair point, I'll update the comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
raise
else:
# This flag will be set to True again if there isn't a savepoint
@@ -245,7 +250,12 @@ def __exit__(self, exc_type, exc_value, traceback):
connection.needs_rollback = True
else:
# Roll back transaction
- connection.rollback()
+ try:
+ connection.rollback()
+ except Error:
+ # Error during rollback means the connection was
+ # closed. Clean up in case the server dropped it.
+ connection.close()
finally:
# Outermost block exit when autocommit was enabled.
View
7 tests/backends/tests.py
@@ -664,13 +664,6 @@ def test_cursor_contextmanager_closing(self):
self.assertIsInstance(cursor, CursorWrapper)
self.assertTrue(cursor.closed)
-
-class IsUsableTests(TransactionTestCase):
- # Avoid using a regular TestCase because Django really dislikes closing
- # the database connection inside a transaction at this point (#21202).
-
- available_apps = []
-
# Unfortunately with sqlite3 the in-memory test database cannot be closed.
@skipUnlessDBFeature('test_db_allows_multiple_connections')
def test_is_usable_after_database_disconnects(self):
Please sign in to comment.
Something went wrong with that request. Please try again.