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
test_threading leaked [38, 38, 38] references, sum=114 #84330
Comments
New changeset 53e4c91 by Dong-hee Na in branch 'master': This change introduced a reference leak: $ ./python -m test -R 3:3 test_threading -m test.test_threading.SubinterpThreadingTests.test_threads_join_2
0:00:00 load avg: 1.52 Run tests sequentially
0:00:00 load avg: 1.52 [1/1] test_threading
beginning 6 repetitions
123456
......
test_threading leaked [19, 19, 19] references, sum=57
test_threading leaked [12, 12, 12] memory blocks, sum=36
test_threading failed == Tests result: FAILURE == 1 test failed: Total duration: 768 ms |
Attached test_leak.py is enough to reproduce the leak. It's related to subinterpreters and the _abc module. |
I'm not sure why, but trigger explicitly a second GC collection fix the issue. diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 2d5cb0ff78..d20ae01238 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1295,6 +1295,7 @@ finalize_interp_clear(PyThreadState *tstate)
/* Trigger a GC collection on subinterpreters*/
if (!is_main_interp) {
_PyGC_CollectNoFail();
+ _PyGC_CollectNoFail();
}
finalize_interp_types(tstate, is_main_interp); |
FYI, |
gc: collectable <type 0x7fc033c3cbd0> is not collected for the first time. |
Running |
In _abcmodule_exec(), when _abc_data type is created, it's created with refcnt=3:
type_traverse() visits tp_dict and tp_mro, so it's fine. In Py_EndInterpreter(), PyInterpreterState_Clear() clears os.register_atfork() callbacks which were one of the last references to _abc_data type. The first call to _PyGC_CollectNoFail() destroys _abc_data *instances* but not the _abc_data type. The following patch works around the issue: diff --git a/Modules/_abc.c b/Modules/_abc.c
index 1efc98bf72..410dbcf96a 100644
--- a/Modules/_abc.c
+++ b/Modules/_abc.c
@@ -54,6 +54,7 @@ typedef struct {
static int
abc_data_traverse(_abc_data *self, visitproc visit, void *arg)
{
+ Py_VISIT(Py_TYPE(self));
Py_VISIT(self->_abc_registry);
Py_VISIT(self->_abc_cache);
Py_VISIT(self->_abc_negative_cache); $ ./python -m test -R 3:3 test_threading -m test.test_threading.SubinterpThreadingTests.test_threads_join_2
(...)
Tests result: SUCCESS I'm not sure why Py_VISIT(Py_TYPE(self)) is needed. Maybe it's a regression caused by commit 364f0b0 of bpo-35810. It sems like the GC doesn't take in account that instances of types allocated on the heap (if type->tp_flags has the Py_TPFLAGS_HEAPTYPE flag) hold a strong refeference to the type (PyObject.ob_type). I created bpo-40217: "The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type". |
Wow Thank you for the summary :) |
Pablo told me that this change is not correct: the overriden traverse function must call PyType_Type.tp_traverse (parent method). |
This issue is now fixed. I tested manually: I confirm that it does fix the test_threading leak, so I close the issue. $ ./python -m test -R 3:3 test_threading
(...)
== Tests result: SUCCESS == 1 test OK. Total duration: 1 min 14 sec |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: