GeezerVison is showing github.com — typing.py: builtin LRU caches worsen leaks that exist in other code · Issue #98253 · python/cpython
typing.py: builtin LRU caches worsen leaks that exist in other code #98253DescriptionBug report
I would like to report a refleak issue involving typing.py. The issue is that it internally uses LRU caches to cache certain type-related lookups, and these caches are not cleaned up when the Python interpreter shuts down. This causes leaks that impede software development and debugging of refleaks in general.
This specific part of typing.py has already once been identified as a source of refleaks by @gvanrossum (context: https://bugs.python.org/issue28649).
The following provides a small reproducer via a trivial package (https://github.com/wjakob/typing_repro) that exposes a class named A using nanobind. Why nanobind? It is extremely paranoid about any leaks involving bound types, functions, and instances, and prints warning messages to tell the user about this after the interpreter has shut down (it performs checks following finalization using Py_AtExit()).
preparation:
$ pip install git+https://github.com/wjakob/typing_repro Reproducer:
from typing_repro import A
import pandas
import typing
def test(t: typing.Optional[A] = None):
print(t)Running this yields
nanobind: leaked 1 types!
- leaked type "A"
nanobind: leaked 2 functions!
- leaked function "add"
- leaked function "__init__"
nanobind: this is likely caused by a reference counting issue in the binding code.
Note the import of pandas, which serves the role of a bigger package that uses the typing module and thereby populates the LRU caches. torch (PyTorch) or tensorflow also cause the issue, as does markupsafe, others likely affected as well.
EDIT: The problem that is common to all of these packages is that they leak some of their own types. For example, by Py_INCREFing references to heap types within extension modules. Because these types use typing.py and thereby reference the LRU caches (which are never cleaned up), it causes a flurry of refleaks that cascade into other packages.
Removing the test() function or removing the type annotation fixes the issue. The problem is that declaration causes cache entries to be created that are never cleaned up, even when the interpreter finalizes.
There is another way to avoid the issue: at the bottom of the script, insert
for f in typing._cleanups:
f()
which clears the LRU caches in typing.py. Poof, errors gone. This leads me to suggest the following simple fix, to be added at the end of typing.py:
def _cleanup_handler():
for f in _cleanups:
f()
import atexit as _atexit
_atexit.register(_cleanup_handler)
This will clear the caches and ensure that interpreter finalization can avoid those type annotation-related leaks.
Your environment
- CPython versions tested on: 3.8.10 and 3.10.;7
- Operating system and architecture: Linux and macOS
ActivityJelleZijlstra commented on Oct 14, 2022 I'm not overly familiar with memory management around the shutdown sequence, but shouldn't these caches get collected automatically when the module object is deallocated and its dict is cleared? Or should every function that uses @lru_cache register an atexit handler to clear the cache? gvanrossum commented on Oct 14, 2022 I honestly have no idea, but if this really was a problem I'd also expect to have heard of this before. Possibly nanobind's check runs before the typing module is cleaned up? Note that the OP appears to be the author of nanobind. wjakob commented on Oct 14, 2022 ContributorAuthor More actions What is potentially confusing is that there are two different kinds of atexit() at play in Python.
-
nanobind's exit handler is installed via Py_AtExit(). This is a list of up to 32 function pointers that are run after the interpreter is already fully shut down. Most Python API calls including garbage collection are no longer possible at this point.
Because nanobind maintains some internal data structures for every bound type outside of Python, it knows if its types have all been fully garbage collected. This has been invaluable in tracking down bugs in bindings, and the library is therefore intentionally very noisy upon noticing leaks.
-
Then, there is the Python atexit module that can be registered from within Python. Those run while the interpreter is still alive. The hack/workaround I suggested above uses that principle to clear the LRU caches in typing.py.
In principle, I agree with @JelleZijlstra's sentiment: the references should be known to Python's cyclic GC, which can then clean things up (including LRU caches related to type annotations). But somehow this is not happening. wjakob commented on Oct 14, 2022 ContributorAuthor More actions rhettinger commented on Oct 15, 2022 wjakob commented on Oct 19, 2022 ContributorAuthor More actions @rhettinger: I am not very familiar with the implementation of the LRU cache but tried the following -- please let me know if that's wrong. Specifically, I went into functools.py and commented out the lines
try:
from _functools import _lru_cache_wrapper
except ImportError:
pass
under the assumption that this is what it takes for the cache to switch over to the pure Python version. I observe that it then uses the Python version of the _lru_cache_wrapper function defined in the same file. However, this did not fix the leak:
nanobind: leaked 1 types!
- leaked type "typing_repro.typing_repro_ext.A"
nanobind: leaked 2 functions!
- leaked function "add"
- leaked function "__init__"
nanobind: this is likely caused by a reference counting issue in the binding code.
The issue should be very easy to reproduce with the 5-LOC script in the first post. Does it happen on your end? JelleZijlstra commented on Oct 19, 2022 I can reproduce the nanobind message with your sample script, but the leak apparently no longer happens when I comment out import pandas. pandas is a big pile of code, so perhaps whatever leak nanobind detects is actually in pandas? wjakob commented on Oct 19, 2022 ContributorAuthor More actions It happens with other big packages as well (for example, try PyTorch or Tensorflow). What I can see is that the LRU caches are filled with lots of data when pandas is imported, and that seems to play a role. I could see how pandas maybe has some reference counting issues internally, but I don't see how that would cause my own types to no longer be deallocated by typing.py.
The caches are definitely implicated in some form -- adding the following code to the reproducer fixes the issue for example:
import atexit
def cleanup():
import typing
for cleanup_func in typing._cleanups:
cleanup_func()
atexit.register(cleanup)
JelleZijlstra commented on Oct 19, 2022 I tried to narrow it down by removing parts of pandas's __init__.py, and found that the reported leak goes away when the pandas._libs.json module is no longer imported. That's a Cython module with no apparent relevance to types; I think its code is here. wjakob commented on Oct 20, 2022 ContributorAuthor More actions Ok, I think I found a smoking gun.
Here is another, much smaller extension, that also produces a type leak:
from typing_repro import A
import markupsafe
import typing
def test(t: typing.Optional[A] = None):
print(t)produces the dreaded
nanobind: leaked 1 types!
- leaked type "typing_repro.typing_repro_ext.A"
nanobind: leaked 2 functions!
- leaked function "__init__"
- leaked function "add"
nanobind: this is likely caused by a reference counting issue in the binding code.
The markupsafe extension contains a pure Python portion defining a markupsafe.Markup class, which includes type annotations that presumably trigger the LRU cache. But there is also a native C extension markupsafe._speedups, which is tiny, self-contained, and doesn't even use Cython (phew!).
The problem can be tied down to a function that is called as part of the module initialization:
static PyObject* markup;
static int
init_constants(void)
{
PyObject *module;
/* import markup type so that we can mark the return value */
module = PyImport_ImportModule("markupsafe");
if (!module)
return 0;
markup = PyObject_GetAttrString(module, "Markup");
Py_DECREF(module);
return 1;
}
/* removed lots of stuff ... */
PyMODINIT_FUNC
PyInit__speedups(void)
{
if (!init_constants())
return NULL;
return PyModule_Create(&module_definition);
}What's the problem? This extension module internally stashes a reference to the Markup class so that it can be used for internal purposes later on. It also implicitly increases the reference count via the PyObject_GetAttrString method. Adding Py_DECREF(markup); causes the warning to go away. So this is at the end of the day a simple reference leak within the markupsafe package.
HOWEVER: I don't think it is reasonable that this also causes other heap types to leak all across the Python interpreter. And it is specifically typing and the builtin LRU cache that are to blame for this. wjakob commented on Oct 20, 2022 ContributorAuthor More actions I see are potential workarounds. First, the LRU cache used by typing could potentially be modified so that it holds weak references that don't cause leaks across the program. However, I am not sure how practical that is.
The second one is the band-aid I suggested. The following could be added somewhere in typing.py and would take care of the leak at shutdown time.
def _cleanup_handler():
for f in _cleanups:
f()
import atexit as _atexit
_atexit.register(_cleanup_handler)
JelleZijlstra commented on Oct 21, 2022 Good catch! So the leak chain is markupsafe.Markup -> one of its method has an Optional[str] annotation -> Optional[str] resolves to an instance of typing._UnionGenericAlias -> typing._UnionGenericAlias.__getitem__ holds on to its LRU cache -> that cache contains a reference to typing_repro.A.
I suppose we could put in the cleanup you recommend, but wouldn't it be better to fix the third-party packages that have this bug? Fidget-Spinner commented on Oct 25, 2022 Just to reiterate so that I don't get lost in the weeds here:
- typing.py's LRU cache itself doesn't leak references. It just holds onto other things which may leak references or is held by said other things.
- Those other leaky things may hold a circular reference to typing.py's LRU cache through indirect means that are naturally found in typing.py.
- So it's proposed to break the reference cycle on typing.py's end, instead of getting the leaky extension to do it.
Is this correct? Reacted by Raymond Hettinger wjakob commented on Oct 25, 2022 ContributorAuthor More actions Yes, that summarizes things well. I would add that typing is an elementary part of the Python ecosystem, which includes many compiled extension with individually benign type leaks that now cause issues through the interaction with typing. Expecting them to be fixed instead would seem to me like an impossible game of whack-a-mole. changed the title [-]typing.py: builtin LRU caches leak references[/-] [+]typing.py: builtin LRU caches worsen leaks that exist in other code[/+] on Oct 25, 2022JelleZijlstra commented on Nov 5, 2022 Note that the repro no longer works at the moment because nanobind 0.0.8 contains a workaround for this leak. I proposed wjakob/typing_repro#3 to "fix" it. In the meantime, to use the repro, first install nanobind 0.0.7 and the package's other build requirements, then install typing_repro as in the OP above but with --no-build-isolation. added a commit that references this issue on Nov 30, 2022gh-98253: Break potential reference cycles in external code worsened … AlexWaygood commented on Nov 30, 2022 Thanks for the PR and for being patient, @wjakob wjakob commented on Dec 2, 2022 ContributorAuthor More actions Thanks for accepting my patch :-) Reacted by Alex Waygood and Ken Jin
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|