Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#30171 closed Cleanup/optimization (fixed)

Fix DatabaseError threading error during servers tests

Reported by: Jon Dufresne Owned by: nobody
Component: Database layer (models, ORM) Version: 2.2
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

When running tests with Python warnings enabled, tests.servers.tests.LiveServerPort produces a warning of the form:

Exception in thread Thread-1:
Traceback (most recent call last):
  File "/usr/lib64/python3.7/threading.py", line 917, in _bootstrap_inner
    self.run()
  File "django/test/testcases.py", line 1399, in run
    connections.close_all()
  File "django/db/utils.py", line 224, in close_all
    connection.close()
  File "django/db/backends/sqlite3/base.py", line 244, in close
    self.validate_thread_sharing()
  File "django/db/backends/base/base.py", line 531, in validate_thread_sharing
    % (self.alias, self._thread_ident, _thread.get_ident())
django.db.utils.DatabaseError: DatabaseWrapper objects created in a thread can only be used in that same thread. The object with alias 'default' was created in thread id 139685002331648 and this is thread id 139684747486976.

This occurs because multiple classes set DatabaseWrapper.allow_thread_sharing to false upon test tear down. The base test class and the temporarily created test class. In other words, nesting the setup/teardown of DatabaseWrapper.allow_thread_sharing isn't always handled cleanly.

Change History (8)

comment:1 by Jon Dufresne, 6 years ago

Has patch: set

comment:2 by Tim Graham, 6 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Tests aren't passing.

comment:3 by Tim Graham, 6 years ago

Description: modified (diff)
Patch needs improvement: unset
Summary: Fix Python warning during LiveServerPortFix DatabaseError threading error during servers tests
Triage Stage: AcceptedReady for checkin
Version: master2.2

The warning appeared after 8c775391b78b2a4a2b57c5e89ed4888f36aada4b, so we'll backport to stable/2.2.x.

comment:4 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: newclosed

In 76990cb:

Fixed #30171 -- Fixed DatabaseError in servers tests.

Made DatabaseWrapper thread sharing logic reentrant. Used a reference
counting like scheme to allow nested uses.

The error appeared after 8c775391b78b2a4a2b57c5e89ed4888f36aada4b.

comment:5 by Tim Graham <timograham@…>, 6 years ago

In 37cc6a9d:

[2.2.x] Fixed #30171 -- Fixed DatabaseError in servers tests.

Made DatabaseWrapper thread sharing logic reentrant. Used a reference
counting like scheme to allow nested uses.

The error appeared after 8c775391b78b2a4a2b57c5e89ed4888f36aada4b.
Backport of 76990cbbda5d93fda560c8a5ab019860f7efaab7 from master.

comment:6 by Chris Jerdonek, 4 years ago

I'm seeing in Django 2.2.17 what looks like the reappearance of #22414 after it being fixed in 1.11. There don't seem to be a whole lot of changes in the LiveServerTestCase-related code between those two versions of Django.

I'm wondering if it's possible this commit explains it:
https://github.com/django/django/commit/37cc6a9dce3354cd37f23ee972bc25b0e5cebd5c

For example, was it intentional that this SQLite-specific condition (if conn.vendor == 'sqlite' and conn.is_in_memory_db():) was removed from _tearDownClassInternal(), even though it's still in setUpClass()? Line here:
https://github.com/django/django/commit/37cc6a9dce3354cd37f23ee972bc25b0e5cebd5c#diff-7833da5b45a68d00834388d97dd5c4413e3796497c7bc5e0cc2621b08a2d0df1L1483

Also, for reasons of symmetry with setUpClass(), it doesn't seem like those connection restore lines should have been moved beneath the if hasattr(cls, 'server_thread'): condition in _tearDownClassInternal(). Basically, it looks like a couple lines were deleted, perhaps by accident.

Last edited 4 years ago by Chris Jerdonek (previous) (diff)

comment:7 by Carlton Gibson, 4 years ago

Hi Chris.

Can I ask you to open a new issue with a reproduce please? Ideally an adjustment to the test (such as those modified in 76990cbbda5d93fda560c8a5ab019860f7efaab7) or otherwise a sample project or similar. Without spending a lot of time, from your comment it's not easy to see what was missing from the coverage there. Thanks!

comment:8 by Chris Jerdonek, 4 years ago

Okay, I created a ticket for the symmetry issue here: #32417. (The regression issue I created a ticket for here: #32416. I found it wasn't due to this issue, but to a different one: #20238.)

Note: See TracTickets for help on using tickets.
Back to Top