GeezerVison is showing github.com — bpo-42972: Fix sqlite3 traverse/clear by erlend-aasland · Pull Request #26452 · python/cpython
Conversation
erlend-aasland
commented
May 29, 2021
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
GH-26104 had some issues:
- members were not visited/cleared in correct order
- weak refs were cleared in
Py_tp_clear iso. Py_tp_dealloc
- some cursor members were not visited
This PR fixes these issues.
https://bugs.python.org/issue42972
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Whenever you have time, Pablo & Victor.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
🤖 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.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
PPC64LE Fedora Stable Refleaks PR: 2:59:32 load avg: 1.00 running: test_asyncio (2 hour 39 min), unrelated issue.
buildbot/AMD64 Windows8.1 Refleaks PR:
0:25:45 load avg: 4.38 [ 44/427/1] test_concurrent_futures failed (20 min 5 sec) -- running: test_cmd_line (25 min 44 sec), test_tools (44.7 sec), test_marshal (20 min 22 sec)
beginning 6 repetitions
123456
.....Warning -- threading_cleanup() failed to cleanup 2 threads (count: 2, dangling: 3)
Warning -- Dangling thread: <Thread(ThreadPoolExecutor-254_0, started 3616)>
Warning -- Dangling thread: <Thread(ThreadPoolExecutor-254_1, started 2112)>
Warning -- Dangling thread: <_MainThread(MainThread, started 5044)>
test test_concurrent_futures failed -- Traceback (most recent call last):
File "D:\buildarea\pull_request.ware-win81-release.refleak\build\lib\test\test_concurrent_futures.py", line 905, in test_idle_thread_reuse
self.assertEqual(len(executor._threads), 1)
AssertionError: 2 != 1
test_asyncio
Warning -- Unraisable exception
Traceback (most recent call last):
File "D:\buildarea\pull_request.ware-win81-release.refleak\build\lib\asyncio\windows_events.py", line 434, in select
self._poll(timeout)
RuntimeError: <_overlapped.Overlapped object at 0x00000022989E54E0> still has pending operation at deallocation, the process may crash
4 re-run tests:
test_asyncio test_concurrent_futures test_ssl test_threading
Total duration: 3 hour 2 min
Tests result: FAILURE then SUCCESS
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
@@ -42,9 +42,9 @@ row_clear(pysqlite_Row *self) |
|
|
static int
|
|
|
row_traverse(pysqlite_Row *self, visitproc visit, 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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@erlend-aasland: While reviewing traverse/clear functions, I noticed surprising code in pysqlite_cache_dealloc():
if (!self->factory) {
/* constructor failed, just get out of here */
return;
}
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 ;-)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
While reviewing traverse/clear functions, I noticed surprising code in pysqlite_cache_dealloc():
if (!self->factory) {
/* constructor failed, just get out of here */
return;
}
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 ;-)
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)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot x86 Gentoo Installed with X 3.x has failed when building commit d1124b0.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/464/builds/292) and take a look at the build logs.
- Check if the failure is related to this commit (d1124b0) or if it is a false positive.
- If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.
You can take a look at the buildbot page here:
https://buildbot.python.org/all/#builders/464/builds/292
Failed tests:
Failed subtests:
- test_incremental_editing - idlelib.idle_test.test_colorizer.ColorDelegatorTest
Summary of the results of the build (if available):
== Tests result: FAILURE then FAILURE ==
412 tests OK.
1 test failed:
test_idle
14 tests skipped:
test_asdl_parser test_check_c_globals test_clinic test_ctypes
test_devpoll test_gdb test_ioctl test_kqueue test_msilib
test_startfile test_winconsoleio test_winreg test_winsound
test_zipfile64
1 re-run test:
test_idle
Total duration: 26 min 43 sec
Click to see traceback logs
Traceback (most recent call last):
File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.11/idlelib/idle_test/tkinter_testing_utils.py", line 54, in new_test_method
raise exception
File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.11/idlelib/idle_test/tkinter_testing_utils.py", line 38, in after_callback
next(test_generator)
File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.11/idlelib/idle_test/test_colorizer.py", line 573, in test_incremental_editing
eq(text.tag_nextrange('BUILTIN', '1.0'), ('1.0', '1.3'))
AssertionError: Tuples differ: () != ('1.0', '1.3')
Traceback (most recent call last):
File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.11/idlelib/idle_test/tkinter_testing_utils.py", line 54, in new_test_method
raise exception
File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.11/idlelib/idle_test/tkinter_testing_utils.py", line 38, in after_callback
next(test_generator)
File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.11/idlelib/idle_test/test_colorizer.py", line 569, in test_incremental_editing
eq(text.tag_nextrange('KEYWORD', '1.0'), ('1.0', '1.2'))
AssertionError: Tuples differ: () != ('1.0', '1.2')
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
vstinner
vstinner approved these changes
pablogsal
Awaiting requested review from pablogsal
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|