SimpleTestCase does not block database connections in threads

[Daniel Hahler]

Due to ConnectionHandler's connections being thread-local [1] new connections will be used in new threads, which then do not have been patched for disallowed methods [2].

Given test_simpletestcase.py:

import threading

from django.db import connection
from django.test import SimpleTestCase


class MySimpleTestCase(SimpleTestCase):
    def test_this(self):
        try:
            with connection.cursor() as cursor:
                cursor.execute("SELECT 1")
            raise Exception("should have failed")
        except AssertionError:
            pass

        res = []

        def thread_func():
            res.append(1)
            try:
                with connection.cursor() as cursor:
                    cursor.execute("SELECT 1")
                raise Exception("should have failed")
            except AssertionError:
                pass
            res.append(2)

        t = threading.Thread(target=thread_func)
        t.start()
        t.join()
        assert res == [1, 2], res

./manage.py test test_simpletestcase.py fails like this:

Exception in thread Thread-1:
Traceback (most recent call last):
  File "/usr/lib/python3.9/threading.py", line 973, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.9/threading.py", line 910, in run
    self._target(*self._args, **self._kwargs)
  File "…/test_simpletestcase.py", line 23, in thread_func
    raise Exception("should have failed")
Exception: should have failed
F
======================================================================
FAIL: test_this (test_simpletestcase.MySimpleTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "…/test_simpletestcase.py", line 31, in test_this
    assert res == [1, 2], res
AssertionError: [1]

----------------------------------------------------------------------
Ran 1 test in 0.006s

FAILED (failures=1)

(Note that there is some handling of connection.settings_dict for workers of the test runner, which is only slightly related: ​https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/test/runner.py#L327-L335)

A possible solution might be to use the existing ​connection_created signal to raise an exception when a connections was created (although that would happen only after the fact - a new pre-connect signal could be used/added for this).

Given that the test DB names are not prefixed with test_ with SimpleTestCase you might accidentally change the production DB from within your tests when something like a ThreadPoolExecutor is being used when mixing sync with async etc.

Note: pytest-django monkeypatches django.db.backends.base.base.BaseDatabaseWrapper.ensure_connection to block DB access, which appears to work better in this regard (across threads).

1: ​https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/utils/connection.py#L41
2: ​https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/test/testcases.py#L183

[Carlton Gibson]

Hi Daniel, thanks for this — Nice report. (Was momentarily confused by the footnotes linking to the initial commit, e.g. [1] :)

Yes, I think SimpleTestCase likely should do the right thing here.

[David Wobrock]

Hey Sulabh,

I see you assigned yourself the ticket, that's great!
I've started a ​PR some months ago, feel free to check it out if it can give you some inspiration :)

[Sulabh Katila]

Hello David,

Thank you for getting the ball rolling on this feature! I've looked at the PR, and it's impressive. I'll definitely use it as inspiration to continue the work. If you have any specific insights or tips based on your progress so far, please feel free to share.
I'm looking forward to working on this together!

Replying to David Wobrock:

Hey Sulabh,

I see you assigned yourself the ticket, that's great!
I've started a ​PR some months ago, feel free to check it out if it can give you some inspiration :)

[Sulabh Katila]

Hi David,

I've been grappling with this issue for a while now, and I haven't made much progress. Given your expertise in this area, I'm reaching out for some much-needed guidance.

To address this, I've been contemplating the idea of implementing a "pre-signal" solution. However, I must admit that I'm currently stuck, and I'm unsure about how to move forward.

I had initially considered intervening at the BaseDataBaseWrapper.connect() level, but I haven't been able to find a clear and effective path for implementation.

Since you have experience in related areas of this issue, I was hoping you could provide some insights. What should I be looking for, or are there alternative approaches that might be more suitable?

Thank you!

Replying to David Wobrock:

Hey Sulabh,

I see you assigned yourself the ticket, that's great!
I've started a ​PR some months ago, feel free to check it out if it can give you some inspiration :)

[David Wobrock]

I do not think I have that much experience.

How did you implement the "pre-signal" solution? Do you have an open PR/branch to check out the code :)

And did you have a look on the ​patching method approach, and play around with it?
I feel like it could be a working path - it mainly requires some test fixing.

Replying to Sulabh Katila:

Hi David,

I've been grappling with this issue for a while now, and I haven't made much progress. Given your expertise in this area, I'm reaching out for some much-needed guidance.

To address this, I've been contemplating the idea of implementing a "pre-signal" solution. However, I must admit that I'm currently stuck, and I'm unsure about how to move forward.

I had initially considered intervening at the BaseDataBaseWrapper.connect() level, but I haven't been able to find a clear and effective path for implementation.

Since you have experience in related areas of this issue, I was hoping you could provide some insights. What should I be looking for, or are there alternative approaches that might be more suitable?

Thank you!

Replying to David Wobrock:

Hey Sulabh,

I see you assigned yourself the ticket, that's great!
I've started a ​PR some months ago, feel free to check it out if it can give you some inspiration :)

[David Wobrock]

Hey Sulabh,

I took a few more minutes to dig into the issue and I think we are close to a fix :)
djangoci.com is down at the time of writing, so I'm missing the complete test suite, but it looks quite promising for now! We just need to fix a few tests.

=> ​PR

[David Wobrock]

I picked up the existing patch and opened a new ​PR.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 8fb0be35:

Fixed #33277 -- Disallowed database connections in threads in SimpleTestCase.