Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-42972: Fix sqlite3 traverse/clear#26452
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
erlend-aasland commented May 29, 2021 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
erlend-aasland commented May 29, 2021
Whenever you have time, Pablo & Victor. |
bce1100 to 2e7f768Compare2e7f768 to 62bbb5cComparebedevere-bot commented May 29, 2021
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 62bbb5c 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
vstinner commented May 31, 2021
PPC64LE Fedora Stable Refleaks PR: buildbot/AMD64 Windows8.1 Refleaks PR:
|
vstinner 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. I checked that all Python objects are visited in traverse functions, and that clear functions clear all objects visited in traverse functions.
The coding style issue is not worth to require another revision.
| } | ||
| staticint | ||
| row_traverse(pysqlite_Row*self, visitprocvisit, void*arg) |
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.
nitpick: row.c is the only file where clear is declared before traverse.
miss-islington commented May 31, 2021
Thanks @erlend-aasland for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
(cherry picked from commit d1124b0) Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
bedevere-bot commented May 31, 2021
GH-26461 is a backport of this pull request to the 3.10 branch. |
vstinner commented May 31, 2021
@erlend-aasland: While reviewing traverse/clear functions, I noticed surprising code in pysqlite_cache_dealloc(): PyObject_GC_UnTrack() is not called if pysqlite_cache_init() failed, IMO it's a bug: the object is tracked in new, not in init. Note: pysqlite_cache_init() doesn't use Py_SETREF() or Py_CLEAR() and so leaks references if called manually twice. Well, nobody should do that ;-) |
erlend-aasland commented May 31, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Yes, those are unfortunate issues :) However, they'll soon (hopefully) begone! Hint: #24203 😃 (You're welcome to take a look at that, if you find time) |
bedevere-bot commented May 31, 2021
|
erlend-aasland commented May 31, 2021
Thanks for reviewing, @vstinner ! |
GH-26104 had some issues:
Py_tp_cleariso.Py_tp_deallocThis PR fixes these issues.
https://bugs.python.org/issue42972