Skip to content

FIX: do not cache exceptions #25470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/matplotlib/_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 2 additions & 0 deletions lib/matplotlib/cbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
18 changes: 11 additions & 7 deletions lib/matplotlib/font_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -128,6 +129,7 @@
'sans',
}

_ExceptionProxy = namedtuple('_ExceptionProxy', ['klass', 'message'])

# OS Font paths
try:
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down
25 changes: 24 additions & 1 deletion lib/matplotlib/tests/test_font_manager.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from io import BytesIO, StringIO
import gc
import multiprocessing
import os
from pathlib import Path
Expand All @@ -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

Expand Down Expand Up @@ -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")