test__xxsubinterpreters test_atexit test_capi test_threading are leaking references

[pablogsal]

BPO	38997
Nosy	@vstinner, @pablogsal
PRs	bpo-38997: Revert commits 6e157fa and 5df5dbe #17509 bpo-38858: Fix ref leak in pycore_interp_init() #17512

^{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:

assignee = 'https://github.com/pablogsal'
closed_at = <Date 2019-12-08.21:05:48.392>
created_at = <Date 2019-12-08.15:35:11.987>
labels = ['type-bug', 'tests', '3.9']
title = 'test__xxsubinterpreters test_atexit test_capi test_threading are leaking references'
updated_at = <Date 2019-12-08.21:37:19.649>
user = 'https://github.com/pablogsal'

bugs.python.org fields:

activity = <Date 2019-12-08.21:37:19.649>
actor = 'pablogsal'
assignee = 'pablogsal'
closed = True
closed_date = <Date 2019-12-08.21:05:48.392>
closer = 'vstinner'
components = ['Tests']
creation = <Date 2019-12-08.15:35:11.987>
creator = 'pablogsal'
dependencies = []
files = []
hgrepos = []
issue_num = 38997
keywords = ['patch']
message_count = 8.0
messages = ['358006', '358007', '358008', '358009', '358029', '358030', '358032', '358034']
nosy_count = 2.0
nosy_names = ['vstinner', 'pablogsal']
pr_nums = ['17509', '17512']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue38997'
versions = ['Python 3.9']

[pablogsal]

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

[pablogsal]

Bisecting points to:

commit 81fe5bd
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().

[pablogsal]

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.

[pablogsal]

I will try to work on an alternative fix meanwhile, but I need to search for what exactly is leaking first.

[vstinner]

New changeset 080ee5a by Victor Stinner in branch 'master':
bpo-38858: Fix ref leak in pycore_interp_init() (GH-17512)
080ee5a

[vstinner]

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 080ee5a.

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...

[vstinner]

I validated that my commit 080ee5a 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