Code

Opened 5 years ago

Closed 13 months ago

#10744 closed Bug (fixed)

transaction.leave_transaction_management leaks

Reported by: Glenn Owned by: aaugustin
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

leave_transaction_management removes threads from state, but not dirty. This will cause a leak as threads are destroyed, and causes the set_clean and set_dirty sanity check ("This code isn't under transaction management") to not always trigger. It may also cause problems if created threads get the same thread identifier as a previous thread. Attached patch cleans up the transaction state when exiting transaction management; obviously, TLS would be a cleaner and more reliable solution.

This patch causes a failure in regressiontests.cache.tests.DBCacheTests:

======================================================================
ERROR: test_add (regressiontests.cache.tests.DBCacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/glenn/django/tests/regressiontests/cache/tests.py", line 142, in test_add
    result = self.cache.add("addkey1", "newvalue")
  File "/home/glenn/django/django/core/cache/backends/db.py", line 45, in add
    return self._base_set('add', key, value, timeout)
  File "/home/glenn/django/django/core/cache/backends/db.py", line 68, in _base_set
    transaction.rollback()
  File "/home/glenn/django/django/db/transaction.py", line 180, in rollback
    set_clean()
  File "/home/glenn/django/django/db/transaction.py", line 114, in set_clean
    raise TransactionManagementError("This code isn't under transaction management")
TransactionManagementError: This code isn't under transaction management

I don't know what _base_set should be doing, but it seems like this error is intended to be triggered in this situation.

Attachments (1)

cleanup-transaction-state-on-exit.diff (698 bytes) - added by Glenn 5 years ago.

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by Glenn

comment:1 Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by SmileyChris

  • Severity set to Normal
  • Type set to Bug

comment:3 Changed 3 years ago by patchhammer

  • Easy pickings unset
  • Patch needs improvement set

cleanup-transaction-state-on-exit.diff fails to apply cleanly on to trunk

comment:4 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:5 Changed 13 months ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Status changed from new to assigned

comment:6 Changed 13 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 14cddf51c5f001bb426ce7f7a83fdc52c8d8aee9:

Merged branch 'database-level-autocommit'.

Fixed #2227: atomic supports nesting.
Fixed #6623: commit_manually is deprecated and atomic doesn't suffer from this defect.
Fixed #8320: the problem wasn't identified, but the legacy transaction management is deprecated.
Fixed #10744: the problem wasn't identified, but the legacy transaction management is deprecated.
Fixed #10813: since autocommit is enabled, it isn't necessary to rollback after errors any more.
Fixed #13742: savepoints are now implemented for SQLite.
Fixed #13870: transaction management in long running processes isn't a problem any more, and it's documented.
Fixed #14970: while it digresses on transaction management, this ticket essentially asks for autocommit on PostgreSQL.
Fixed #15694: atomic supports nesting.
Fixed #17887: autocommit makes it impossible for a connection to stay "idle of transaction".

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.