From 4bdeb9046c51cec03965633e3fe29b946b3e7cb0 Mon Sep 17 00:00:00 2001 From: Mikhail Koviazin Date: Wed, 18 Dec 2024 18:01:26 +0100 Subject: [PATCH] connection: add a pointer to os.getpid to ConnectionPool This commit adds a local pointer to `os.getpid` to `ConnectionPool` class to mitigate the `AttributeError` that occurs on Python 3.13+ on interpreter exit. This is caused by the fact that interpreter calls `__del__` on variables on exit, but `os` is being destroyed before the `valkey.Valkey` instance. It could be easily reproduced with: >>> import valkey >>> r = valkey.Valkey(host="localhost", port=6379) >>> exit() Exception ignored in: Traceback (most recent call last): File "/home/mikhail.koviazin/workspace/valkey-py-clean/valkey/client.py", line 521, in __del__ File "/home/mikhail.koviazin/workspace/valkey-py-clean/valkey/client.py", line 536, in close File "/home/mikhail.koviazin/workspace/valkey-py-clean/valkey/connection.py", line 1179, in disconnect File "/home/mikhail.koviazin/workspace/valkey-py-clean/valkey/connection.py", line 1083, in _checkpid TypeError: 'NoneType' object is not callable Having a local pointer to that function keeps `os.getpid()` accessible during interpreter shutdown. Fixes #158 Signed-off-by: Mikhail Koviazin --- valkey/connection.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/valkey/connection.py b/valkey/connection.py index 6e2861cf..5141785a 100644 --- a/valkey/connection.py +++ b/valkey/connection.py @@ -1011,6 +1011,20 @@ def __init__( self.connection_kwargs = connection_kwargs self.max_connections = max_connections + # We need to preserve the pointer to os.getpid because Valkey class + # contains a __del__ method that causes the call chain: + # 1. Valkey.close() + # 2. ConnectionPool.disconnect() + # 3. ConnectionPool._checkpid() + # 4. os.getpid() + # + # If os.getpid is garbage collected before Valkey, then the __del__ + # method will raise an AttributeError when trying to call os.getpid. + # It wasn't an issue in practice until Python REPL was reworked in 3.13 + # to collect all globals at the end of the session, which caused + # os.getpid to be garbage collected before Valkey. + self._getpid = os.getpid + # a lock to protect the critical section in _checkpid(). # this lock is acquired when the process id changes, such as # after a fork. during this time, multiple threads in the child @@ -1080,14 +1094,14 @@ def _checkpid(self) -> None: # seconds to acquire _fork_lock. if _fork_lock cannot be acquired in # that time it is assumed that the child is deadlocked and a # valkey.ChildDeadlockedError error is raised. - if self.pid != os.getpid(): + if self.pid != self._getpid(): acquired = self._fork_lock.acquire(timeout=5) if not acquired: raise ChildDeadlockedError # reset() the instance for the new process if another thread # hasn't already done so try: - if self.pid != os.getpid(): + if self.pid != self._getpid(): self.reset() finally: self._fork_lock.release()