Opened 16 months ago
Closed 16 months ago
#35618 closed Uncategorized (duplicate)
response.close() in a TestCase prematurely closes PostgreSQL connection and leads to psycopg2.InterfaceError
| Reported by: | Anders Kaseorg | Owned by: | |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 5.0 | 
| Severity: | Normal | Keywords: | |
| Cc: | Carlton Gibson, Jon Janzen | Triage Stage: | Unreviewed | 
| Has patch: | yes | Needs documentation: | no | 
| Needs tests: | no | Patch needs improvement: | no | 
| Easy pickings: | no | UI/UX: | no | 
Description
If I call response.close() on an HTTP response from the test client within a django.test.TestCase, Django incorrectly closes the active PostgreSQL connection, causing future test requests to fail with psycopg2.InterfaceError: connection already closed.
This happens because HttpResponseBase.close sends signals.request_finished, which is handled by django.db.close_old_connections, which calls BaseDatabaseWrapper.close_if_unusable_or_obsolete, which observes self.get_autocommit() != self.settings_dict["AUTOCOMMIT"] (False != True) and closes the connection.
Any connection that’s within a transaction (such as the one opened by TestCase) will have get_autocommit() == False. It seems very wrong that the finishing of any request automatically triggers all connections in transactions to be immediately closed.
Minimal complete test project: https://gist.github.com/andersk/b4da5a106995b4c525595ea204675ee0
$ ./manage.py test
Found 1 test(s).
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
E
======================================================================
ERROR: test (tests.MyTest.test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/base/base.py", line 294, in _cursor
    return self._prepare_cursor(self.create_cursor(name))
                                ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/utils/asyncio.py", line 26, in inner
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/postgresql/base.py", line 332, in create_cursor
    cursor = self.connection.cursor()
             ^^^^^^^^^^^^^^^^^^^^^^^^
psycopg2.InterfaceError: connection already closed
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
  File "/tmp/close-test/tests.py", line 8, in test
    response = self.client.get("/two")
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 1049, in get
    response = super().get(path, data=data, secure=secure, headers=headers, **extra)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 465, in get
    return self.generic(
           ^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 617, in generic
    return self.request(**r)
           ^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 1013, in request
    self.check_exception(response)
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 743, in check_exception
    raise exc_value
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/my_urls.py", line 7, in view
    with connection.cursor():
         ^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/utils/asyncio.py", line 26, in inner
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/base/base.py", line 316, in cursor
    return self._cursor()
           ^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/base/base.py", line 293, in _cursor
    with self.wrap_database_errors:
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/base/base.py", line 294, in _cursor
    return self._prepare_cursor(self.create_cursor(name))
                                ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/utils/asyncio.py", line 26, in inner
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/postgresql/base.py", line 332, in create_cursor
    cursor = self.connection.cursor()
             ^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.InterfaceError: connection already closed
----------------------------------------------------------------------
Ran 1 test in 0.018s
FAILED (errors=1)
Destroying test database for alias 'default'...
      Change History (7)
comment:2 by , 16 months ago
The streaming case is the one that motivated me to call response.close in the first place. Currently there’s no way to get closing_iterator_wrapper to close the response itself without first consuming the entire stream, and leaving it unclosed results in various ResourceWarnings when warnings are enabled.
comment:3 by , 16 months ago
| Cc: | added | 
|---|
We had a similar issue in Channels, which we've addressed by (similarly) disabling the close_old_connections handler during tests. 
We have this down as related to #30448. 
comment:4 by , 16 months ago
| Has patch: | set | 
|---|
comment:6 by , 16 months ago
Should we close as duplicate of #30448, this seems like the exact same problem.
comment:7 by , 16 months ago
| Resolution: | → duplicate | 
|---|---|
| Status: | new → closed | 
Yes, agreed. Duplicate of #30448. 
In order to prevent this problem from happening under normal circumstances
django.test.Clientunregisters this signal handler before callingrequest.close.I feel like if you're testing something as low level as
HttpRequestclosing in the context of database connection handling you should be usingClientHandler?I guess that we could possibly make
request.closeidempotent given what's returned bytest.Clientshould already be closed?