-
-
Notifications
You must be signed in to change notification settings - Fork 33.6k
gh-140042: Removing unsafe call to sqlite3_shutdown #141690
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
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
encukou
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.
Thank you.
I don't see a good way to add a test here -- we'd need to inject a failure in the module initialization logic, and then watch undefined behaviour.
If we want to clean up properly, we should call sqlite3_shutdown in e.g. module_free. That's not the case here -- sqlite3 leaks in the “happy” path. Also leaking on error isn't a big issue.
(Presumably we leak a fixed amount of resources per process, so adding proper calls to sqlite3_shutdown isn't a big priority.)
@erlend-aasland, do you want to take a look?
I'll merge around Thursday if there are no objections.
|
Thanks @prithviraj-chaudhuri for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
|
GH-141793 is a backport of this pull request to the 3.14 branch. |
|
Sorry, @prithviraj-chaudhuri and @encukou, I could not cleanly backport this to |
This commit addresses this issue #140042 (comment).
It removes the sqlite3_shutdown call that could cause closing connections for sqlite when used with multiple sub interpreters.