-
Notifications
You must be signed in to change notification settings - Fork 23
fix: thread leak #52
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
fix: thread leak #52
Conversation
Greptile SummaryFixed thread leak in
Confidence Score: 4/5
Important Files Changed
|
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.
2 files reviewed, 2 comments
victorvhs017
left a comment
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.
LGTM!
The SecretsCache was leaking threads because it relied on the Python
__del__method to stop the background cleanup thread, but__del__is not reliably called by the python garbage collector. Each time a client was created, a new cleanup thread was spawned that would never terminate, causing thread counts to grow without bounds in long running applications or applications that create multiple client instances.Users can now optionally call
client.close()or use the client as a context manager (with InfisicalSDKClient(...) as client:) for instant cleanup, but even without explicit cleanup, threads will now self-terminate when the garbage collector reclaims the cache object. This ensures the SDK no longer leaks threads regardless of how it's used.Two new dependencies,
weakrefandatexit, which have been apart of the Python standard library since 2000/2001 respectively. This will continue working even for folks on older Python versions.