Skip to content

Commit

Permalink
FIX: do not cache exceptions
Browse files Browse the repository at this point in the history
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 matplotlib#25406
  • Loading branch information
tacaswell committed Mar 15, 2023
1 parent 370547e commit db0c7c3
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 8 deletions.
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")

0 comments on commit db0c7c3

Please sign in to comment.