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

connection: add a pointer to os.getpid to ConnectionPool #159

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

mkmkme
Copy link
Collaborator

@mkmkme mkmkme commented Dec 18, 2024

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

Pull Request check-list

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?

Description of change

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 valkey-io#158

Signed-off-by: Mikhail Koviazin <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.23%. Comparing base (ebdf9c9) to head (4bdeb90).

Files with missing lines Patch % Lines
valkey/connection.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #159   +/-   ##
=======================================
  Coverage   76.22%   76.23%           
=======================================
  Files         130      130           
  Lines       33946    33947    +1     
=======================================
+ Hits        25877    25878    +1     
  Misses       8069     8069           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mkmkme mkmkme merged commit ce23ae4 into valkey-io:main Dec 18, 2024
94 checks passed
@mkmkme mkmkme added the maintenance dependabot PR's label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance dependabot PR's
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Django runserver throws AttributeError on closing
4 participants