From 200e4dfc40ce0705ab6a3aca906e0a67884235e2 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sat, 8 Feb 2020 08:47:08 -0800 Subject: Stop hiding errors that occur inside __del__ methods If an exception occurs inside the __del__ method, it should be reported to the developer. Not doing so could hide bugs. Python automatically handles exceptions inside __del__ methods, for example: class A: def __del__(self): 1 / 0 A() print("after del") Results in the output: $ python3 ~/blah.py Exception ignored in: Traceback (most recent call last): File "/home/jon/test.py", line 3, in __del__ 1 / 0 ZeroDivisionError: division by zero after del From this example, we can see the bug was not hidden and the code after __del__ still executed. fixes #1281 --- CHANGES | 9 ++++++++- redis/client.py | 16 +++++----------- redis/connection.py | 15 +++------------ 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/CHANGES b/CHANGES index 7fac16f..94c8ccb 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +* (in development) + * Removed exception trapping from __del__ methods. redis-py objects that + hold various resources implement __del__ cleanup methods to release + those resources when the object goes out of scope. This provides a + fallback for when these objects aren't explicitly closed by user code. + Prior to this change any errors encountered in closing these resources + would be hidden from the user. Thanks @jdufresne. #1281 * 3.4.1 * Move the username argument in the Redis and Connection classes to the end of the argument list. This helps those poor souls that specify all @@ -14,7 +21,7 @@ in 3.4.0. This ended up being a bad idea as two separate connection pools be considered equal yet manage a completely separate set of connections. - * HSET command now can accept multiple pairs. HMSET has been marked as + * HSET command now can accept multiple pairs. HMSET has been marked as deprecated now. Thanks to @laixintao #1271 * 3.4.0 * Allow empty pipelines to be executed if there are WATCHed keys. diff --git a/redis/client.py b/redis/client.py index fe697f6..9cb60f5 100755 --- a/redis/client.py +++ b/redis/client.py @@ -3375,13 +3375,10 @@ class PubSub(object): self.reset() def __del__(self): - try: - # if this object went out of scope prior to shutting down - # subscriptions, close the connection manually before - # returning it to the connection pool - self.reset() - except Exception: - pass + # if this object went out of scope prior to shutting down + # subscriptions, close the connection manually before + # returning it to the connection pool + self.reset() def reset(self): if self.connection: @@ -3730,10 +3727,7 @@ class Pipeline(Redis): self.reset() def __del__(self): - try: - self.reset() - except Exception: - pass + self.reset() def __len__(self): return len(self.command_stack) diff --git a/redis/connection.py b/redis/connection.py index b5e3fb1..dbcf332 100755 --- a/redis/connection.py +++ b/redis/connection.py @@ -288,10 +288,7 @@ class PythonParser(BaseParser): self._buffer = None def __del__(self): - try: - self.on_disconnect() - except Exception: - pass + self.on_disconnect() def on_connect(self, connection): "Called when the socket connects" @@ -370,10 +367,7 @@ class HiredisParser(BaseParser): self._buffer = bytearray(socket_read_size) def __del__(self): - try: - self.on_disconnect() - except Exception: - pass + self.on_disconnect() def on_connect(self, connection): self._sock = connection._sock @@ -533,10 +527,7 @@ class Connection(object): return pieces def __del__(self): - try: - self.disconnect() - except Exception: - pass + self.disconnect() def register_connect_callback(self, callback): self._connect_callbacks.append(callback) -- cgit v1.2.1