Opened 20 months ago

Closed 3 weeks ago

#36027 closed Bug (fixed)

ASGI: Dead persistent postgres connections are not closed when the database is accessed in response_for_exception

Reported by: ruijafreitas Owned by: Carlton Gibson
Component: Error reporting Version: 5.1
Severity: Normal Keywords: sync_to_async thread_sensitive, async
Cc: ruijafreitas, Carlton Gibson 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

This problem is similar to ticket:31905

How to reproduce:

  1. Create a 404 or other exception template that use a db connection.
  2. Make a first request to a url that return for example a 404.
  3. Restart your db server, forcing connections to be closed.
  4. Visit again the same url and you will receive the error "the connection is closed".

Traceback:

 File "django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "django/core/handlers/base.py", line 235, in _get_response_async
    callback, callback_args, callback_kwargs = self.resolve_request(request)
  File "django/core/handlers/base.py", line 313, in resolve_request
    resolver_match = resolver.resolve(request.path_info)
  File "django/urls/resolvers.py", line 705, in resolve
    raise Resolver404({"tried": tried, "path": new_path})
OperationalError: the connection is closed
  File "django/db/backends/base/base.py", line 298, in _cursor
    return self._prepare_cursor(self.create_cursor(name))
  File "django/utils/asyncio.py", line 26, in inner
    return func(*args, **kwargs)
  File "django/db/backends/postgresql/base.py", line 429, in create_cursor
    cursor = self.connection.cursor()
  File "psycopg/connection.py", line 213, in cursor
    self._check_connection_ok()
  File "psycopg/_connection_base.py", line 524, in _check_connection_ok
    raise e.OperationalError("the connection is closed")
OperationalError: the connection is closed
  File "django/core/handlers/exception.py", line 164, in get_exception_response
    response = callback(request, exception=exception)
  File "django/utils/decorators.py", line 188, in _view_wrapper
    result = _process_exception(request, e)
  File "django/utils/decorators.py", line 186, in _view_wrapper
    response = view_func(request, *args, **kwargs)
  File "django/views/defaults.py", line 64, in page_not_found
    body = template.render(context, request)
  File "django/template/backends/django.py", line 107, in render
    return self.template.render(context)
  File "django/template/base.py", line 169, in render
    with context.bind_template(self):
  File "contextlib.py", line 137, in __enter__
    return next(self.gen)
  File "django/template/context.py", line 256, in bind_template
    context = processor(self.request)
  File "core/context_processors.py", line 15, in global_settings
    site_settings = SiteSettings.get_instance(request=request)
  File "core/models.py", line 180, in get_instance
    return SiteSettings.for_request(request)
  File "wagtail/contrib/settings/models.py", line 127, in for_request
    site = Site.find_for_request(request)
  File "wagtail/models/sites.py", line 157, in find_for_request
    site = Site._find_for_request(request)
  File "wagtail/models/sites.py", line 167, in _find_for_request
    site = get_site_for_hostname(hostname, port)
  File "wagtail/models/sites.py", line 23, in get_site_for_hostname
    sites = list(
  File "django/db/models/query.py", line 400, in __iter__
    self._fetch_all()
  File "django/db/models/query.py", line 1928, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "django/db/models/query.py", line 91, in __iter__
    results = compiler.execute_sql(
  File "django/db/models/sql/compiler.py", line 1572, in execute_sql
    cursor = self.connection.cursor()
  File "django/utils/asyncio.py", line 26, in inner
    return func(*args, **kwargs)
  File "django/db/backends/base/base.py", line 320, in cursor
    return self._cursor()
  File "django/db/backends/base/base.py", line 297, in _cursor
    with self.wrap_database_errors:
  File "django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "django/db/backends/base/base.py", line 298, in _cursor
    return self._prepare_cursor(self.create_cursor(name))
  File "django/utils/asyncio.py", line 26, in inner
    return func(*args, **kwargs)
  File "django/db/backends/postgresql/base.py", line 429, in create_cursor
    cursor = self.connection.cursor()
  File "psycopg/connection.py", line 213, in cursor
    self._check_connection_ok()
  File "psycopg/_connection_base.py", line 524, in _check_connection_ok
    raise e.OperationalError("the connection is closed")

This problem could be solved, removing the argument thread_sensitive from sync_to_async in convert_exception_to_response function Link to Repo File.

With this change, the thread used will be the same of the outer task, where the connections were already checked in request_start signal by close_old_connections.

Change History (14)

comment:1 by Sarah Boyce, 20 months ago

Cc: Carlton Gibson added
Triage Stage: UnreviewedAccepted

Thank you ruijafreitas - replicated

comment:2 by Ishita Srivastava, 20 months ago

Owner: set to Ishita Srivastava
Status: newassigned

comment:3 by Rahul Yadav, 16 months ago

Owner: changed from Ishita Srivastava to Rahul Yadav

comment:4 by Carlton Gibson, 16 months ago

Keywords: async added

comment:5 by Rahul Yadav, 14 months ago

Owner: Rahul Yadav removed
Status: assignednew

comment:6 by YashRaj1506, 14 months ago

Owner: set to YashRaj1506
Status: newassigned

in reply to:  1 comment:7 by YashRaj1506, 10 months ago

Replying to Sarah Boyce:

Thank you ruijafreitas - replicated

Hey Sarah, i am trying to prepare a patch for this issue, but i am not able to recreate this issue on my system. Can you guide me how you recreated it? i did follow the steps mentioned above for recreation but its not showing the same output as mentioned above.

comment:8 by Jacob Walls, 10 months ago

Did you try the steps outlined in #31905, or did you start an ASGI server some other way?

comment:9 by George Margaritis, 8 months ago

Hey, is this issue actively worked on, or is it up for grabs?

We’ve been experiencing it in production and would love to have it merged upstream.

comment:10 by Jacob Walls, 8 months ago

It's up for grabs -- a patch with tests would be helpful. If you start working on it, please set yourself in the owner field. Thanks!

comment:11 by George Margaritis, 8 months ago

Owner: changed from YashRaj1506 to George Margaritis

comment:12 by Carlton Gibson, 3 weeks ago

Has patch: set
Owner: changed from George Margaritis to Carlton Gibson

comment:13 by Jacob Walls, 3 weeks ago

Triage Stage: AcceptedReady for checkin

comment:14 by Jacob Walls <jacobtylerwalls@…>, 3 weeks ago

Resolution: fixed
Status: assignedclosed

In 6fc8150:

Fixed #36027 -- Made error response rendering thread-sensitive.

Error response handling frequently makes use of the database, and so
must use the request specific connection sensitive worker thread.

Previously response_for_exception was dispatched on the event loop's
default executor. This potentially leads to pool exhaustion, and
deadlock, when using database pooling option, or a large number of
permanently held open database connections when not using pooling.

Connections held open were not subject to cleanup. If closed server
side, the reported "connection is closed" would be seen until
application restart.

Moving error response rendering to the request specific connection
sensitive worker thread resolves both of these issues.

The thread_sensitive=False was set in
e17ee4468875077b90b70bb6a589ebad7493f757 to maintain the prior
behaviour when asgiref switched the thread_sensitive default to
True. There was no specific reason for the non-tread-sensitive
behaviour beyond that. (With hindsight it was always incorrect for
the reasons here.)

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