GC operates out of global runtime state.

[ericsnowcurrently]

BPO	36854
Nosy	@vstinner, @phsilva, @ericsnowcurrently, @pablogsal, @miss-islington
PRs	bpo-36854: Move GC runtime state from _PyRuntimeState to PyInterpreterState. #13219 bpo-36854: Clear the current thread later #17279 bpo-36854: gcmodule.c gets its state from tstate #17285 bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState #17287 bpo-36854: Fix refleak in subinterpreter #17331 [3.8] bpo-36854: Fix reference counter in PyInit__testcapi() #17338 [3.7] bpo-36854: Fix reference counter in PyInit__testcapi() (GH-17338) #17339 Revert "bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState (GH-17287)" #17455 bpo-46070: Revert "bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState (GH-17287)" #30564 [3.10] bpo-46070: Revert "bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState (GH-17287)" #30565 [3.9] bpo-46070: Revert "bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState (GH-17287)" #30566

^{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/ericsnowcurrently'
closed_at = <Date 2019-11-22.13:00:22.999>
created_at = <Date 2019-05-08.17:00:25.446>
labels = ['expert-subinterpreters', '3.9']
title = 'GC operates out of global runtime state.'
updated_at = <Date 2022-01-12.23:02:12.811>
user = 'https://github.com/ericsnowcurrently'

bugs.python.org fields:

activity = <Date 2022-01-12.23:02:12.811>
actor = 'vstinner'
assignee = 'eric.snow'
closed = True
closed_date = <Date 2019-11-22.13:00:22.999>
closer = 'vstinner'
components = ['Subinterpreters']
creation = <Date 2019-05-08.17:00:25.446>
creator = 'eric.snow'
dependencies = []
files = []
hgrepos = []
issue_num = 36854
keywords = ['patch']
message_count = 18.0
messages = ['341911', '357048', '357053', '357061', '357062', '357150', '357151', '357160', '357263', '357269', '357273', '357275', '357276', '357328', '357329', '357795', '357796', '358355']
nosy_count = 5.0
nosy_names = ['vstinner', 'phsilva', 'eric.snow', 'pablogsal', 'miss-islington']
pr_nums = ['13219', '17279', '17285', '17287', '17331', '17338', '17339', '17455', '30564', '30565', '30566']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue36854'
versions = ['Python 3.9']

[ericsnowcurrently]

(also see bpo-24554)

We need to move GC state from _PyRuntimeState to PyInterpreterState.

[vstinner]

New changeset 9da7430 by Victor Stinner in branch 'master':
bpo-36854: Clear the current thread later (GH-17279)
9da7430

[vstinner]

New changeset 67e0de6 by Victor Stinner in branch 'master':
bpo-36854: gcmodule.c gets its state from tstate (GH-17285)
67e0de6

[vstinner]

New changeset 7247407 by Victor Stinner in branch 'master':
bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState (GH-17287)
7247407

[vstinner]

It's now done in the future Python 3.9!

[vstinner]

I reopen the issue, the change introduced a reference leak :-( Example:

$ ./python -m test -R 3:3 test_atexit -m test.test_atexit.SubinterpreterTest.test_callbacks_leak
0:00:00 load avg: 1.12 Run tests sequentially
0:00:00 load avg: 1.12 [1/1] test_atexit
beginning 6 repetitions
123456
......
test_atexit leaked [3988, 3986, 3988] references, sum=11962
test_atexit leaked [940, 939, 940] memory blocks, sum=2819
test_atexit failed

== Tests result: FAILURE ==

1 test failed:
test_atexit

Total duration: 466 ms
Tests result: FAILURE

It seems like each _testcapi.run_in_subinterp("pass") call leaks 3988 references.

I tried tracemalloc to see where the memory allocation are done, but tracemalloc reports a single Python line: the _testcapi.run_in_subinterp() call...

I tried to follow the increase of references using a watchpoint in gdb on _Py_RefTotal, but it takes a lot of time to follow each Py_INCREF/Py_DECREF knowning that we are talking aobut around 4,000 references.

[vstinner]

Even if the test is simplified to the following code, it does still leak:

    def test_callbacks_leak(self):
        _testcapi.run_in_subinterp("pass")

[vstinner]

test_atexit leaked [3988, 3986, 3988] references, sum=11962

The following patch fix it:

diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 7591f069b4..f088ef0bce 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1210,6 +1210,15 @@ finalize_interp_clear(PyThreadState *tstate)
 {
     int is_main_interp = _Py_IsMainInterpreter(tstate);
 
+    _PyImport_Cleanup(tstate);
+
+    /* Explicitly break a reference cycle between the encodings module and XXX */
+    PyInterpreterState *interp = tstate->interp;
+    Py_CLEAR(interp->codec_search_path);
+    Py_CLEAR(interp->codec_search_cache);
+    Py_CLEAR(interp->codec_error_registry);
+    _PyGC_CollectNoFail();
+
     /* Clear interpreter state and all thread states */
     PyInterpreterState_Clear(tstate->interp);
 
@@ -1640,7 +1649,6 @@ Py_EndInterpreter(PyThreadState *tstate)
         Py_FatalError("Py_EndInterpreter: not the last thread");
     }
 
-    _PyImport_Cleanup(tstate);
     finalize_interp_clear(tstate);
     finalize_interp_delete(tstate);
 }

Py_NewInterpreter() indirectly calls "import encodings" which calls codecs.register(search_function). This encodings function is stored in interp->codec_search_path and so keeps encodings module dict alive.

_PyImport_Cleanup() removes the last reference to the encodings *module*, but the module deallocator function (module_dealloc()) doesn't clear the dict: it only removes its strong reference to it ("Py_XDECREF(m->md_dict);").

interp->codec_search_path is cleared by PyInterpreterState_Clear() which is called by Py_EndInterpreter(). But it is not enough to clear some objets. I'm not sure if encodings module dict is still alive at this point, but it seems like at least the sys module dict is still alive.

I can push my workaround which manually "break a reference cycle" (really? which one?), but I may be interested to dig into this issue to check if we cannot find a better design.

_PyImport_Cleanup() and _PyModule_Clear() functions are fragile. They implement smart heuristics to attempt to keep Python functional as long as possible *and* try to clear everything. The intent is to be able to log warnings and exceptions during the Python shutdown, for example.

The problem is that the heuristic keeps some objects alive longer than expected. For example, I would expect that _PyImport_Cleanup() not only calls sys.modules.clear(), but also clears the dict of each module (module.__dict__.clear()). It doesn't, and I'm not sure why.