Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite loop and memory leak caused by the omit_exceptions decorator #658

Open
HosseyNJF opened this issue Mar 7, 2023 · 2 comments
Open
Labels

Comments

@HosseyNJF
Copy link

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?

@HosseyNJF HosseyNJF added the bug label Mar 7, 2023
@HosseyNJF
Copy link
Author

Possibly related: #630

@some1ataplace
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants