Skip to content

Commit

Permalink
connection: add a pointer to os.getpid to ConnectionPool
Browse files Browse the repository at this point in the history
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: <function Valkey.__del__ at 0x7f29d084e5c0>
	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 <[email protected]>
  • Loading branch information
mkmkme committed Dec 18, 2024
1 parent ebdf9c9 commit 4bdeb90
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions valkey/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 4bdeb90

Please sign in to comment.