msg358006 - (view) |
Author: Pablo Galindo Salgado (pablogsal) *  |
Date: 2019-12-08 15:35 |
Similar to https://bugs.python.org/issue38962, test__xxsubinterpreters test_atexit test_capi test_threading are leaking references. Example: https://buildbot.python.org/all/#/builders/158/builds/10 https://buildbot.python.org/all/#/builders/16/builds/11 https://buildbot.python.org/all/#/builders/157/builds/11 test__xxsubinterpreters test_atexit test_capi test_threading == Tests result: FAILURE then FAILURE == 386 tests OK. 10 slowest tests: - test_multiprocessing_spawn: 26 min 23 sec - test_mailbox: 23 min 17 sec - test_asyncio: 20 min 25 sec - test_venv: 14 min 54 sec - test_concurrent_futures: 13 min 35 sec - test_zipfile: 11 min 10 sec - test_regrtest: 9 min 34 sec - test_distutils: 9 min 19 sec - test_compileall: 9 min 9 sec - test_lib2to3: 5 min 52 sec 4 tests failed: test__xxsubinterpreters test_atexit test_capi test_threading |
|
|
msg358007 - (view) |
Author: Pablo Galindo Salgado (pablogsal) *  |
Date: 2019-12-08 15:35 |
Bisecting points to: commit 81fe5bd3d78f9bb955f8255404d99df27a31c36a Author: Victor Stinner <vstinner@python.org> Date: Fri Dec 6 02:43:30 2019 +0100 bpo-38858: new_interpreter() reuses _PySys_Create() (GH-17481) new_interpreter() now calls _PySys_Create() to create a new sys module isolated from the main interpreter. It now calls _PySys_InitCore() and _PyImport_FixupBuiltin(). init_interp_main() now calls _PySys_InitMain(). |
|
|
msg358008 - (view) |
Author: Pablo Galindo Salgado (pablogsal) *  |
Date: 2019-12-08 15:36 |
Per or policy, I will initiate the revert of the commit (and any dependent commit) and will merge if an alternative fix is not done in 1-2 days. |
|
|
msg358009 - (view) |
Author: Pablo Galindo Salgado (pablogsal) *  |
Date: 2019-12-08 15:39 |
I will try to work on an alternative fix meanwhile, but I need to search for what exactly is leaking first. |
|
|
msg358029 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-12-08 20:56 |
New changeset 080ee5a88406fb68aaab741145cd5d2a7c5f2ad6 by Victor Stinner in branch 'master': bpo-38858: Fix ref leak in pycore_interp_init() (GH-17512) https://github.com/python/cpython/commit/080ee5a88406fb68aaab741145cd5d2a7c5f2ad6 |
|
|
msg358030 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-12-08 21:03 |
This reference leak is not new :-) It exists since at least Python 2.7. Extract of Python 2.7, Python/pythonrun.c: sysmod = _PySys_Init(); if (sysmod == NULL) Py_FatalError("Py_Initialize: can't initialize sys"); interp->sysdict = PyModule_GetDict(sysmod); There is a missing Py_DECREF(sysmod). It was the same bug here, except that the code was deeply refactored in the meanwhile. I fixed the reference leak in commit 080ee5a88406fb68aaab741145cd5d2a7c5f2ad6. Next question: why did the buildbot turn red? Well, at Python exit, there are like 18k references which are never decremented at Python exit: $ ./python -X showrefcount -c pass [18562 refs, 6494 blocks] Previously, the subinterpreter sys module somehow shared references with the main interpreter. With my latest changes, the subinterpreter better isolates its own sys module from the main interpreter, and so the very old bug suddenyl is "releaved". Getting subinterpreter "right" requires to fix all these very old bugs, one by one... |
|
|
msg358032 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-12-08 21:05 |
I validated that my commit 080ee5a88406fb68aaab741145cd5d2a7c5f2ad6 fixed the leaks, so I closed PR 17509 and I close this issue. Thanks for the bug report Pablo! I was also awaiting Refleak buildbot results! $ ./python -m test -R 3:3 -j0 test__xxsubinterpreters test_atexit test_capi test_threading (...) All 4 tests OK. Total duration: 1 min 4 sec Tests result: SUCCESS |
|
|
msg358034 - (view) |
Author: Pablo Galindo Salgado (pablogsal) *  |
Date: 2019-12-08 21:37 |
Thanks for the fix, Victor! |
|
|