| msg365557 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2020-04-02 01:54 |
| New changeset 53e4c91725083975598350877e2ed8e2d0194114 by Dong-hee Na in branch 'master': bpo-40077: Convert _abc module to use PyType_FromSpec() (GH-19202) https://github.com/python/cpython/commit/53e4c91725083975598350877e2ed8e2d0194114 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: test_threading Total duration: 768 ms Tests result: FAILURE |
|
|
| msg365559 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2020-04-02 02:14 |
| Attached test_leak.py is enough to reproduce the leak. It's related to subinterpreters and the _abc module. |
|
|
| msg365560 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2020-04-02 02:42 |
| 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); |
|
|
| msg365566 - (view) |
Author: Dong-hee Na (corona10) * |
Date: 2020-04-02 03:19 |
| FYI, first gc collect 772 secondary gc collect 4 |
|
|
| msg365695 - (view) |
Author: Dong-hee Na (corona10) * |
Date: 2020-04-03 12:15 |
| gc: collectable <type 0x7fc033c3cbd0> gc: collectable <dict 0x104d79830> gc: collectable <tuple 0x104d725a0> gc: collectable <builtin_function_or_method 0x104d79890> is not collected for the first time. |
|
|
| msg365705 - (view) |
Author: Dong-hee Na (corona10) * |
Date: 2020-04-03 15:52 |
| Running from abc import ABCMeta on the subinterpreter makes same size of leak. |
|
|
| msg365912 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2020-04-07 16:36 |
| New changeset 9cc3ebd7e04cb645ac7b2f372eaafa7464e16b9c by Victor Stinner in branch 'master': bpo-40149: Implement traverse in _abc._abc_data (GH-19412) https://github.com/python/cpython/commit/9cc3ebd7e04cb645ac7b2f372eaafa7464e16b9c |
|
|
| msg365913 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2020-04-07 16:50 |
| In _abcmodule_exec(), when _abc_data type is created, it's created with refcnt=3: * 1 strong reference (normal) * +1 ref from tp_dict['__new__'] slot * +1 ref from tp_mro 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 364f0b0f19cc3f0d5e63f571ec9163cf41c62958 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". |
|
|
| msg365915 - (view) |
Author: Dong-hee Na (corona10) * |
Date: 2020-04-07 16:51 |
| Wow Thank you for the summary :) |
|
|
| msg365938 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2020-04-07 20:41 |
| > bpo-40149: Implement traverse in _abc._abc_data (GH-19412) Pablo told me that this change is not correct: the overriden traverse function must call PyType_Type.tp_traverse (parent method). |
|
|
| msg367415 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2020-04-27 12:58 |
| > 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". 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 Tests result: SUCCESS |
|
|