jazzband/django-redis

Infinite loop and memory leak caused by the `omit_exceptions` decorator

HosseyNJF opened this issue · 2 comments

Describe the bug
When the Redis server is down, a ConnectionError is raised. The omit_exceptions decorator will check the ignore_exceptions settings and if it is set to False, it will re-raise the __cause__ of the raised exception, which is a ConnectionInterruped with a cause of redis.ConnectionError. But somehow the __context__ of the redis error is equal to the ConnectionInterruped exception itself. This causes an infinite loop in the django.views.debug.technical_500_response method that tries to print the traceback but finds itself in a never-ending chain of causes and contexts.

To Reproduce
Steps to reproduce the behavior:

  1. Stop the Redis server.
  2. Set DEBUG = True and DJANGO_REDIS_IGNORE_EXCEPTIONS = False.
  3. Try to access the cache in any view.
  4. Watch as the process hangs and eventually gets OOM killed.

Expected behavior
I expected to simply see a ConnectionInterrupted exception in my console and in my response because the connection failed.

Environment (please complete the following information):

  • Python version: 3.9
  • Django Redis Version: 5.2.0
  • Django Version: 2.2
  • redis Version: irrelevant
  • redis-py Version: 4.5.1

Additional context
The problem only happens when Django's DEBUG = True is set, as it will make Django try to print the traceback. Also, the DJANGO_REDIS_IGNORE_EXCEPTIONS setting must be set to False to force the omit_exceptions decorator to re-raise the exception's __cause__. The problem doesn't seem to happen when python itself is trying to print the traceback, and I don't know why. Maybe they expected someone to raise a circular exception and had measures for that?

Possibly related: #630

Maybe try this. I did not test this.

Modify the omit_exceptions decorator to handle the ConnectionError exception more appropriately. The current behavior of re-raising the exception cause with a ConnectionInterrupted exception and setting the context to the ConnectionInterrupted exception itself is what causes the infinite loop when Django's DEBUG is set to True.

Instead, we can modify the omit_exceptions decorator to directly re-raise the original ConnectionError exception and skip the ConnectionInterrupted exception altogether, since it's an unnecessary step. Here's the updated code for the omit_exceptions decorator:

from functools import wraps
from django.conf import settings
from redis import ConnectionError

def omit_exceptions(fn):
    """
    Decorator that catches ConnectionError exceptions
    if DJANGO_REDIS_IGNORE_EXCEPTIONS is set, logs an error,
    and returns None.
    """
    @wraps(fn)
    def wrapper(args, **kwargs):
        try:
            return fn(args, **kwargs)
        except ConnectionError as e:
            if getattr(settings, 'DJANGO_REDIS_IGNORE_EXCEPTIONS', False):
                msg = 'Ignoring redis cache exception: {}'.format(e)
                if hasattr(e, 'args') and e.args:
                    msg+= ". {}.".format(e.args[0])
                logger.error(msg)
                return None
            else:
                raise  # Re-raise the original ConnectionError exception
    return wrapper

With this modification to the code, the omit_exceptions decorator will now directly re-raise the original ConnectionError exception when Django's DEBUG is set to True, preventing the infinite loop bug when using redis cache with Django.