Fixed #23074 -- Avoided leaking savepoints in atomic.
Thanks Chow Loong Jin for the report and the initial patch.
This commit is contained in:
parent
53a61d82b3
commit
729e4ae4f0
|
@ -497,6 +497,7 @@ class BaseDatabaseFeatures(object):
|
||||||
can_return_id_from_insert = False
|
can_return_id_from_insert = False
|
||||||
has_bulk_insert = False
|
has_bulk_insert = False
|
||||||
uses_savepoints = False
|
uses_savepoints = False
|
||||||
|
can_release_savepoints = False
|
||||||
can_combine_inserts_with_and_without_auto_increment_pk = False
|
can_combine_inserts_with_and_without_auto_increment_pk = False
|
||||||
|
|
||||||
# If True, don't use integer foreign keys referring to, e.g., positive
|
# If True, don't use integer foreign keys referring to, e.g., positive
|
||||||
|
|
|
@ -182,6 +182,7 @@ class DatabaseFeatures(BaseDatabaseFeatures):
|
||||||
requires_explicit_null_ordering_when_grouping = True
|
requires_explicit_null_ordering_when_grouping = True
|
||||||
allows_auto_pk_0 = False
|
allows_auto_pk_0 = False
|
||||||
uses_savepoints = True
|
uses_savepoints = True
|
||||||
|
can_release_savepoints = True
|
||||||
atomic_transactions = False
|
atomic_transactions = False
|
||||||
supports_column_check_constraints = False
|
supports_column_check_constraints = False
|
||||||
|
|
||||||
|
|
|
@ -50,6 +50,7 @@ class DatabaseFeatures(BaseDatabaseFeatures):
|
||||||
has_select_for_update_nowait = True
|
has_select_for_update_nowait = True
|
||||||
has_bulk_insert = True
|
has_bulk_insert = True
|
||||||
uses_savepoints = True
|
uses_savepoints = True
|
||||||
|
can_release_savepoints = True
|
||||||
supports_tablespaces = True
|
supports_tablespaces = True
|
||||||
supports_transactions = True
|
supports_transactions = True
|
||||||
can_introspect_ip_address_field = True
|
can_introspect_ip_address_field = True
|
||||||
|
|
|
@ -118,6 +118,10 @@ class DatabaseFeatures(BaseDatabaseFeatures):
|
||||||
def uses_savepoints(self):
|
def uses_savepoints(self):
|
||||||
return Database.sqlite_version_info >= (3, 6, 8)
|
return Database.sqlite_version_info >= (3, 6, 8)
|
||||||
|
|
||||||
|
@cached_property
|
||||||
|
def can_release_savepoints(self):
|
||||||
|
return self.uses_savepoints
|
||||||
|
|
||||||
@cached_property
|
@cached_property
|
||||||
def supports_stddev(self):
|
def supports_stddev(self):
|
||||||
"""Confirm support for STDDEV and related stats functions
|
"""Confirm support for STDDEV and related stats functions
|
||||||
|
|
|
@ -219,6 +219,9 @@ class Atomic(object):
|
||||||
except DatabaseError:
|
except DatabaseError:
|
||||||
try:
|
try:
|
||||||
connection.savepoint_rollback(sid)
|
connection.savepoint_rollback(sid)
|
||||||
|
# The savepoint won't be reused. Release it to
|
||||||
|
# minimize overhead for the database server.
|
||||||
|
connection.savepoint_commit(sid)
|
||||||
except Error:
|
except Error:
|
||||||
# If rolling back to a savepoint fails, mark for
|
# If rolling back to a savepoint fails, mark for
|
||||||
# rollback at a higher level and avoid shadowing
|
# rollback at a higher level and avoid shadowing
|
||||||
|
@ -249,6 +252,9 @@ class Atomic(object):
|
||||||
else:
|
else:
|
||||||
try:
|
try:
|
||||||
connection.savepoint_rollback(sid)
|
connection.savepoint_rollback(sid)
|
||||||
|
# The savepoint won't be reused. Release it to
|
||||||
|
# minimize overhead for the database server.
|
||||||
|
connection.savepoint_commit(sid)
|
||||||
except Error:
|
except Error:
|
||||||
# If rolling back to a savepoint fails, mark for
|
# If rolling back to a savepoint fails, mark for
|
||||||
# rollback at a higher level and avoid shadowing
|
# rollback at a higher level and avoid shadowing
|
||||||
|
|
|
@ -403,3 +403,25 @@ class AtomicMiscTests(TransactionTestCase):
|
||||||
pass
|
pass
|
||||||
# Must not raise an exception
|
# Must not raise an exception
|
||||||
transaction.atomic(Callable())
|
transaction.atomic(Callable())
|
||||||
|
|
||||||
|
@skipUnlessDBFeature('can_release_savepoints')
|
||||||
|
def test_atomic_does_not_leak_savepoints_on_failure(self):
|
||||||
|
# Regression test for #23074
|
||||||
|
|
||||||
|
# Expect an error when rolling back a savepoint that doesn't exist.
|
||||||
|
# Done outside of the transaction block to ensure proper recovery.
|
||||||
|
with self.assertRaises(Error):
|
||||||
|
|
||||||
|
# Start a plain transaction.
|
||||||
|
with transaction.atomic():
|
||||||
|
|
||||||
|
# Swallow the intentional error raised in the sub-transaction.
|
||||||
|
with six.assertRaisesRegex(self, Exception, "Oops"):
|
||||||
|
|
||||||
|
# Start a sub-transaction with a savepoint.
|
||||||
|
with transaction.atomic():
|
||||||
|
sid = connection.savepoint_ids[-1]
|
||||||
|
raise Exception("Oops")
|
||||||
|
|
||||||
|
# This is expected to fail because the savepoint no longer exists.
|
||||||
|
connection.savepoint_rollback(sid)
|
||||||
|
|
Loading…
Reference in New Issue