8000 Merge pull request #22935 from tacaswell/auto-backport-of-pr-22002-on… · matplotlib/matplotlib@3653fc1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 3653fc1

Browse files
QuLogicQuLogic
authored
Merge pull request #22935 from tacaswell/auto-backport-of-pr-22002-on-v3.5.x
Backport PR #22002: Fix TkAgg memory leaks and test for memory growth regressions
2 parents a841f5f + ec6fbfb commit 3653fc1

File tree

6 files changed

+85
-19
lines changed

6 files changed

+85
-19
lines changed

‎environment.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ dependencies:
4848
- nbformat!=5.0.0,!=5.0.1
4949
- pandas!=0.25.0
5050
- pikepdf
51+
- psutil
5152
- pydocstyle>=5.1.0
5253
- pytest!=4.6.0,!=5.4.0
5354
- pytest-cov

‎lib/matplotlib/_pylab_helpers.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ def destroy(cls, num):
6565
if hasattr(manager, "_cidgcf"):
6666
manager.canvas.mpl_disconnect(manager._cidgcf)
6767
manager.destroy()
68+
del manager, num
69+
# Full cyclic garbage collection may be too expensive to do on every
70+
# figure destruction, so we collect only the youngest two generations.
71+
# see: https://github.com/matplotlib/matplotlib/pull/3045
6872
gc.collect(1)
6973

7074
@classmethod

‎lib/matplotlib/backends/_backend_tk.py

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -431,11 +431,12 @@ def __init__(self, canvas, num, window):
431431
# to store the DPI, which will be updated by the C code, and the trace
432432
# will handle it on the Python side.
433433
window_frame = int(window.wm_frame(), 16)
434-
window_dpi = tk.IntVar(master=window, value=96,
435-
name=f'window_dpi{window_frame}')
434+
self._window_dpi = tk.IntVar(master=window, value=96,
435+
name=f'window_dpi{window_frame}')
436+
self._window_dpi_cbname = ''
436437
if _tkagg.enable_dpi_awareness(window_frame, window.tk.interpaddr()):
437-
self._window_dpi = window_dpi # Prevent garbage collection.
438-
window_dpi.trace_add('write', self._update_window_dpi)
438+
self._window_dpi_cbname = self._window_dpi.trace_add(
439+
'write', self._update_window_dpi)
439440

440441
self._shown = False
441442

@@ -489,20 +490,26 @@ def destroy(self, *args):
489490
self.canvas._tkcanvas.after_cancel(self.canvas._idle_draw_id)
490491
if self.canvas._event_loop_id:
491492
self.canvas._tkcanvas.after_cancel(self.canvas._event_loop_id)
493+
if self._window_dpi_cbname:
494+
self._window_dpi.trace_remove('write', self._window_dpi_cbname)
492495

493496
# NOTE: events need to be flushed before issuing destroy (GH #9956),
494-
# however, self.window.update() can break user code. This is the
495-
# safest way to achieve a complete draining of the event queue,
496-
# but it may require users to update() on their own to execute the
497-
# completion in obscure corner cases.
497+
# however, self.window.update() can break user code. An async callback
498+
# is the safest way to achieve a complete draining of the event queue,
499+
# but it leaks if no tk event loop is running. Therefore we explicitly
500+
# check for an event loop and choose our best guess.
498501
def delayed_destroy():
499502
self.window.destroy()
500503

501504
if self._owns_mainloop and not Gcf.get_num_fig_managers():
502505
self.window.quit()
503506

504-
# "after idle after 0" avoids 67E6 Tcl error/race (GH #19940)
505-
self.window.after_idle(self.window.after, 0, delayed_destroy)
507+
if cbook._get_running_interactive_framework() == "tk":
508+
# "after idle after 0" avoids Tcl error/race (GH #19940)
509+
self.window.after_idle(self.window.after, 0, delayed_destroy)
510+
else:
511+
self.window.update()
512+
delayed_destroy()
506513

507514
def get_window_title(self):
508515
return self.window.wm_title()

‎lib/matplotlib/tests/test_backend_tk.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,9 @@ def test_func():
4545
)
4646
except subprocess.TimeoutExpired:
4747
pytest.fail("Subprocess timed out")
48-
except subprocess.CalledProcessError:
49-
pytest.fail("Subprocess failed to test intended behavior")
48+
except subprocess.CalledProcessError as e:
49+
pytest.fail("Subprocess failed to test intended behavior\n"
50+
+ str(e.stderr))
5051
else:
5152
# macOS may actually emit irrelevant errors about Accelerated
5253
# OpenGL vs. software OpenGL, so suppress them.
@@ -158,14 +159,12 @@ def test_never_update(): # pragma: no cover
158159

159160
plt.draw() # Test FigureCanvasTkAgg.
160161
fig.canvas.toolbar.configure_subplots() # Test NavigationToolbar2Tk.
162+
# Test FigureCanvasTk filter_destroy callback
163+
fig.canvas.get_tk_widget().after(100, plt.close, fig)
161164

162165
# Check for update() or update_idletasks() in the event queue, functionally
163166
# equivalent to tkinter.Misc.update.
164-
# Must pause >= 1 ms to process tcl idle events plus extra time to avoid
165-
# flaky tests on slow systems.
166-
plt.pause(0.1)
167-
168-
plt.close(fig) # Test FigureCanvasTk filter_destroy callback
167+
plt.show(block=True)
169168

170169
# Note that exceptions would be printed to stderr; _isolated_tk_test
171170
# checks them.

‎lib/matplotlib/tests/test_backends_interactive.py

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,10 @@ def _test_thread_impl():
195195
future = ThreadPoolExecutor().submit(fig.canvas.draw)
196196
plt.pause(0.5) # flush_events fails here on at least Tkagg (bpo-41176)
197197
future.result() # Joins the thread; rethrows any exception.
198-
plt.close()
199-
fig.canvas.flush_events() # pause doesn't process events after close
198+
plt.close() # backend is responsible for flushing any events here
199+
if plt.rcParams["backend"].startswith("WX"):
200+
# TODO: debug why WX needs this only on py3.8
201+
fig.canvas.flush_events()
200202

201203

202204
_thread_safe_backends = _get_testable_interactive_backends()
@@ -413,3 +415,55 @@ def _lazy_headless():
413415
@pytest.mark.backend('QtAgg', skip_on_importerror=True)
414416
def test_lazy_linux_headless():
415417
proc = _run_helper(_lazy_headless, timeout=_test_timeout, MPLBACKEND="")
418+
419+
420+
# The source of this function gets extracted and run in another process, so it
421+
# must be fully self-contained.
422+
def _test_figure_leak():
423+
import gc
424+
import sys
425+
426+
import psutil
427+
from matplotlib import pyplot as plt
428+
# Second argument is pause length, but if zero we should skip pausing
429+
t = float(sys.argv[1])
430+
p = psutil.Process()
431+
432+
# Warmup cycle, this reasonably allocates a lot
433+
for _ in range(2):
434+
fig = plt.figure()
435+
if t:
436+
plt.pause(t)
437+
plt.close(fig)
438+
mem = p.memory_info().rss
439+
gc.collect()
440+
441+
for _ in range(5):
442+
fig = plt.figure()
443+
if t:
444+
plt.pause(t)
445+
plt.close(fig)
446+
gc.collect()
447+
growth = p.memory_info().rss - mem
448+
449+
print(growth)
450+
451+
452+
# TODO: "0.1" memory threshold could be reduced 10x by fixing tkagg
453+
@pytest.mark.parametrize("env", _get_testable_interactive_backends())
454+
@pytest.mark.parametrize("time_mem", [(0.0, 2_000_000), (0.1, 30_000_000)])
455+
def test_figure_leak_20490(env, time_mem):
456+
pytest.importorskip("psutil", reason="psutil needed to run this test")
457+
458+
# We haven't yet directly identified the leaks so test with a memory growth
459+
# threshold.
460+
pause_time, acceptable_memory_leakage = time_mem
461+
if env["MPLBACKEND"] == "macosx":
462+
acceptable_memory_leakage += 10_000_000
463+
464+
result = _run_helper(
465+
_test_figure_leak, str(pause_time), timeout=_test_timeout, **env
466+
)
467+
468+
growth = int(result.stdout)
469+
assert growth <= acceptable_memory_leakage

‎requirements/testing/all.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
certifi
44
coverage<6.3
5+
psutil
56
pytest!=4.6.0,!=5.4.0
67
pytest-cov
78
pytest-rerunfailures

0 commit comments

Comments
 (0)
0