-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Exception ignored in: <function Connection.__del__ at 0x7f11449b2af0> #1339
Comments
👍 Seeing this issue aswell. Happens when a SIGTERM is fired to the worker. Version: Having done a bit of basic investigation it seem to be the use of
I'll dig a bit deeper when I get a chance. |
@jdufresne any thoughts on why this might be happening? In the first scenario, I'm not sure when In the second, the only attribute look up on line 664 is |
I've been seeing this in my own project as well, I can try to dive into it. |
I'm getting this also, when used in conjunction with |
I had a chance to look into this. Here is what I believe is happening: The
Here is the Python warning on this:
IMO, this is a bug in the code using redis-py and not redis-py itself. The library users are responsible for closing resources they have created. In this case, a redis connection. Please add an explicit call in your code to close the redis connection rather than relying on the An improvement to redis-py would be to remove these finalizers entirely. This has been done in PR #1287. In Python, sockets already have a finalizer to close them. So the |
I did notice this, in order to fully close the connection, the following pattern was required. with redis.Redis(host) as conn:
# Do stuff with conn
conn.connection_pool.disconnect() Only then, did the exceptions/warnings stop. The context manager releases the connection back to the connection pool, but doesn't close the connection. |
I've add connection close method in cache class. class RedisCache(BaseCache):
def __init__(self, location, params):
# ...
self.redis = redis.Redis(host, port, db=0)
def close(self):
self.redis.connection_pool.disconnect() And call this method in class IndexUserClientTest(TestCase):
def tearDown(self):
cache.close() This is solved my issue. |
I'm struggling with what to do here. In an ideal world, I agree with @jdufresne that users should be responsible for cleaning up the resources that they allocate. And that's where I'd like to get to. However redis-py 2.x/3.x has historically hidden much of the resource allocation and cleanup from users. This is further complicated by the use of a connection pool. In long running applications (like a web app), resource cleanup is more about returning the connection back to the pool rather than closing the socket, etc. I think getting to a place where users retrieve client objects, pubsub objects, pipeline objects, etc. from a global manager object via context managers is the right way to go. This is a fundamental API change and probably should be a 4.0 goal. Especially since we've already stated that 3.5 will be the last 3.x release and we should most certainly not make such a change in a bugfix version like 3.5.x. I'm leaning towards restoring the try/except clause in the Thoughts? |
+1. This sounds like a good idea to me. |
I've restored the try/except clauses in |
Is 3.5.3 coming out soon? We use a large redis cluster, so there are quite a few of these messages popping up anytime we use the library. |
3.5.3 is out with this fix. |
I am facing this same issue while trying out redis-py (v: 5.2.0) on python (v: 3.13.0). A simple way to reproduce this is:
Packages:
|
Version:
redis-py: 3.5.0
redis: 6.0.1
Description:
After commit 200e4df when exception catching was removed from
__del__
methods. I faced this error after run tests in Django. With previous version of redis-py (3.4.1) everything was OK.The text was updated successfully, but these errors were encountered: