From db0c7c3617809a56b39c517c550b6c2600bfc330 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Wed, 15 Mar 2023 15:37:10 -0400 Subject: [PATCH 1/2] FIX: do not cache exceptions This leads to caching the tracebacks which can keep user's objects in local namespaces alive indefinitely. This can lead to very surprising memory issues for users and will result in incorrect tracebacks. Responsive to #25406 --- lib/matplotlib/font_manager.py | 18 +++++++++------- lib/matplotlib/tests/test_font_manager.py | 25 ++++++++++++++++++++++- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/lib/matplotlib/font_manager.py b/lib/matplotlib/font_manager.py index b0d3c0964408..b4022e76be54 100644 --- a/lib/matplotlib/font_manager.py +++ b/lib/matplotlib/font_manager.py @@ -26,6 +26,7 @@ # - 'light' is an invalid weight value, remove it. from base64 import b64encode +from collections import namedtuple import copy import dataclasses from functools import lru_cache @@ -128,6 +129,7 @@ 'sans', } +_ExceptionProxy = namedtuple('_ExceptionProxy', ['klass', 'message']) # OS Font paths try: @@ -1288,8 +1290,8 @@ def findfont(self, prop, fontext='ttf', directory=None, ret = self._findfont_cached( prop, fontext, directory, fallback_to_default, rebuild_if_missing, rc_params) - if isinstance(ret, Exception): - raise ret + if isinstance(ret, _ExceptionProxy): + raise ret.klass(ret.message) return ret def get_font_names(self): @@ -1440,10 +1442,12 @@ def _findfont_cached(self, prop, fontext, directory, fallback_to_default, fallback_to_default=False) else: # This return instead of raise is intentional, as we wish to - # cache the resulting exception, which will not occur if it was + # cache that it was not found, which will not occur if it was # actually raised. - return ValueError(f"Failed to find font {prop}, and fallback " - f"to the default font was disabled") + return _ExceptionProxy( + ValueError, + f"Failed to find font {prop}, and fallback to the default font was disabled" + ) else: _log.debug('findfont: Matching %s to %s (%r) with score of %f.', prop, best_font.name, best_font.fname, best_score) @@ -1463,9 +1467,9 @@ def _findfont_cached(self, prop, fontext, directory, fallback_to_default, prop, fontext, directory, rebuild_if_missing=False) else: # This return instead of raise is intentional, as we wish to - # cache the resulting exception, which will not occur if it was + # cache that it was not found, which will not occur if it was # actually raised. - return ValueError("No valid font could be found") + return _ExceptionProxy(ValueError, "No valid font could be found") return _cached_realpath(result) diff --git a/lib/matplotlib/tests/test_font_manager.py b/lib/matplotlib/tests/test_font_manager.py index 966539088e42..16b5551193cc 100644 --- a/lib/matplotlib/tests/test_font_manager.py +++ b/lib/matplotlib/tests/test_font_manager.py @@ -1,4 +1,5 @@ from io import BytesIO, StringIO +import gc import multiprocessing import os from pathlib import Path @@ -16,7 +17,7 @@ json_dump, json_load, get_font, is_opentype_cff_font, MSUserFontDirectories, _get_fontconfig_fonts, ft2font, ttfFontProperty, cbook) -from matplotlib import pyplot as plt, rc_context +from matplotlib import pyplot as plt, rc_context, figure as mfigure has_fclist = shutil.which('fc-list') is not None @@ -324,3 +325,25 @@ def test_get_font_names(): assert set(available_fonts) == set(mpl_font_names) assert len(available_fonts) == len(mpl_font_names) assert available_fonts == mpl_font_names + + +def test_donot_cache_tracebacks(): + + class SomeObject: + pass + + def inner(): + x = SomeObject() + fig = mfigure.Figure() + ax = fig.subplots() + fig.text(.5, .5, 'aardvark', family='doesnotexist') + with BytesIO() as out: + with warnings.catch_warnings(): + warnings.filterwarnings('ignore') + fig.savefig(out, format='png') + + inner() + + for obj in gc.get_objects(): + if isinstance(obj, SomeObject): + pytest.fail("object from inner stack still alive") From 01390c53fcc777207a5f2c9b93f1cf8d3799e9c7 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Wed, 15 Mar 2023 15:47:54 -0400 Subject: [PATCH 2/2] MNT: delete references to frame objects This is to help prevent reference cycles between the frames and the locals in that frame. While garbage collection should in most cases take care of this eventually, we can help the interpreter out and break the cycle before we return. --- lib/matplotlib/_api/__init__.py | 2 ++ lib/matplotlib/cbook.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/matplotlib/_api/__init__.py b/lib/matplotlib/_api/__init__.py index f4c390a198f8..e667abd5ff55 100644 --- a/lib/matplotlib/_api/__init__.py +++ b/lib/matplotlib/_api/__init__.py @@ -385,4 +385,6 @@ def warn_external(message, category=None): frame.f_globals.get("__name__", "")): break frame = frame.f_back + # premetively break reference cycle between locals and the frame + del frame warnings.warn(message, category, stacklevel) diff --git a/lib/matplotlib/cbook.py b/lib/matplotlib/cbook.py index 3c97e26f6316..00ef34553d0a 100644 --- a/lib/matplotlib/cbook.py +++ b/lib/matplotlib/cbook.py @@ -67,6 +67,8 @@ def _get_running_interactive_framework(): if frame.f_code in codes: return "tk" frame = frame.f_back + # premetively break reference cycle between locals and the frame + del frame macosx = sys.modules.get("matplotlib.backends._macosx") if macosx and macosx.event_loop_is_running(): return "macosx"