-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Avoid stacktrace on process exit in Client.__del__() #3397
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
Avoid stacktrace on process exit in Client.__del__() #3397
Conversation
Note that I unfortunately cannot provide an easy way to reproduce the issue; I can only reproduce it using our own codebase. I suspect this is due to the fact that we load a much larger amount of code than what a synthetic / minimal reproduction attempt would. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @noirbee, thank you for your contribution! I have requested some changes to be added. Once done, I think we will be ready to merge it.
Client.close() may call ConnectionPool.release() or ConnectionPool.disconnect(); both methods may end up calling os.getpid() (through ConnectionPool._checkpid() or threading.Lock() (through ConnectionPool.reset()). As mentioned in the Python documentation [1], at interpreter shutdown, module globals (in this case, the os and threading module references) may be deleted or set to None before __del__() methods are called. This causes an AttributeError to be raised when trying to run e.g. os.getpid(); while the error is ignored by the interpreter, the traceback is still printed out to stderr. Closes redis#3014 [1] https://docs.python.org/3/reference/datamodel.html#object.__del__
f9d8d6e
to
da04843
Compare
Thank you for reviewing @petyaslavova , I've updated the code to catch all exceptions and just |
Client.close() may call ConnectionPool.release() or ConnectionPool.disconnect(); both methods may end up calling os.getpid() (through ConnectionPool._checkpid() or threading.Lock() (through ConnectionPool.reset()). As mentioned in the Python documentation [1], at interpreter shutdown, module globals (in this case, the os and threading module references) may be deleted or set to None before __del__() methods are called. This causes an AttributeError to be raised when trying to run e.g. os.getpid(); while the error is ignored by the interpreter, the traceback is still printed out to stderr. Closes redis#3014 [1] https://docs.python.org/3/reference/datamodel.html#object.__del__ Co-authored-by: petyaslavova <petya.slavova@redis.com>
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
Client.close() may call ConnectionPool.release() or ConnectionPool.disconnect(); both methods may end up calling os.getpid() (through ConnectionPool._checkpid() or threading.Lock() (through ConnectionPool.reset()). As mentioned in the Python documentation [1], at interpreter shutdown, module globals (in this case, the os and threading module references) may be deleted or set to None before del() methods are called. This causes an AttributeError to be raised when trying to run e.g. os.getpid(); while the error is ignored by the interpreter, the traceback is still printed out to stderr.
Closes #3014
[1] https://docs.python.org/3/reference/datamodel.html#object.__del__