Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1a016f0

Browse filesBrowse files
richardsheridanQuLogic
richardsheridan
and
QuLogic
authored
FIX: TkAgg memory leaks and test for memory growth regressions (#22002)
tkinter variables get cleaned up with normal `destroy` and `gc` semantics but tkinter's implementation of trace is effectively global and keeps the callback object alive until the trace is removed. Additionally extend and clean up the tests. Closes #20490 Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
1 parent 9c590de commit 1a016f0
Copy full SHA for 1a016f0

File tree

Expand file treeCollapse file tree

6 files changed

+86
-19
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+86
-19
lines changed

‎environment.yml

Copy file name to clipboardExpand all lines: environment.yml
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ dependencies:
4747
- nbformat!=5.0.0,!=5.0.1
4848
- pandas!=0.25.0
4949
- pikepdf
50+
- psutil
5051
- pre-commit
5152
- pydocstyle>=5.1.0
5253
- pytest!=4.6.0,!=5.4.0

‎lib/matplotlib/_pylab_helpers.py

Copy file name to clipboardExpand all lines: lib/matplotlib/_pylab_helpers.py
+4Lines 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

Copy file name to clipboardExpand all lines: lib/matplotlib/backends/_backend_tk.py
+17-10Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -423,11 +423,12 @@ def __init__(self, canvas, num, window):
423423
# to store the DPI, which will be updated by the C code, and the trace
424424
# will handle it on the Python side.
425425
window_frame = int(window.wm_frame(), 16)
426-
window_dpi = tk.IntVar(master=window, value=96,
427-
name=f'window_dpi{window_frame}')
426+
self._window_dpi = tk.IntVar(master=window, value=96,
427+
name=f'window_dpi{window_frame}')
428+
self._window_dpi_cbname = ''
428429
if _tkagg.enable_dpi_awareness(window_frame, window.tk.interpaddr()):
429-
self._window_dpi = window_dpi # Prevent garbage collection.
430-
window_dpi.trace_add('write', self._update_window_dpi)
430+
self._window_dpi_cbname = self._window_dpi.trace_add(
431+
'write', self._update_window_dpi)
431432

432433
self._shown = False
433434

@@ -472,20 +473,26 @@ def destroy(self, *args):
472473
self.canvas._tkcanvas.after_cancel(self.canvas._idle_draw_id)
473474
if self.canvas._event_loop_id:
474475
self.canvas._tkcanvas.after_cancel(self.canvas._event_loop_id)
476+
if self._window_dpi_cbname:
477+
self._window_dpi.trace_remove('write', self._window_dpi_cbname)
475478

476479
# NOTE: events need to be flushed before issuing destroy (GH #9956),
477-
# however, self.window.update() can break user code. This is the
478-
# safest way to achieve a complete draining of the event queue,
479-
# but it may require users to update() on their own to execute the
480-
# completion in obscure corner cases.
480+
# however, self.window.update() can break user code. An async callback
481+
# is the safest way to achieve a complete draining of the event queue,
482+
# but it leaks if no tk event loop is running. Therefore we explicitly
483+
# check for an event loop and choose our best guess.
481484
def delayed_destroy():
482485
self.window.destroy()
483486

484487
if self._owns_mainloop and not Gcf.get_num_fig_managers():
485488
self.window.quit()
486489

487-
# "after idle after 0" avoids Tcl error/race (GH #19940)
488-
self.window.after_idle(self.window.after, 0, delayed_destroy)
490+
if cbook._get_running_interactive_framework() == "tk":
491+
# "after idle after 0" avoids Tcl error/race (GH #19940)
492+
self.window.after_idle(self.window.after, 0, delayed_destroy)
493+
else:
494+
self.window.update()
495+
delayed_destroy()
489496

490497
def get_window_title(self):
491498
return self.window.wm_title()

‎lib/matplotlib/tests/test_backend_tk.py

Copy file name to clipboardExpand all lines: lib/matplotlib/tests/test_backend_tk.py
+7-7Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,9 @@ def test_func():
5050
)
5151
except subprocess.TimeoutExpired:
5252
pytest.fail("Subprocess timed out")
53-
except subprocess.CalledProcessError:
54-
pytest.fail("Subprocess failed to test intended behavior")
53+
except subprocess.CalledProcessError as e:
54+
pytest.fail("Subprocess failed to test intended behavior\n"
55+
+ str(e.stderr))
5556
else:
5657
# macOS may actually emit irrelevant errors about Accelerated
5758
# OpenGL vs. software OpenGL, so suppress them.
@@ -146,6 +147,7 @@ def target():
146147
thread.join()
147148

148149

150+
@pytest.mark.backend('TkAgg', skip_on_importerror=True)
149151
@pytest.mark.flaky(reruns=3)
150152
@_isolated_tk_test(success_count=0)
151153
def test_never_update():
@@ -159,14 +161,12 @@ def test_never_update():
159161

160162
plt.draw() # Test FigureCanvasTkAgg.
161163
fig.canvas.toolbar.configure_subplots() # Test NavigationToolbar2Tk.
164+
# Test FigureCanvasTk filter_destroy callback
165+
fig.canvas.get_tk_widget().after(100, plt.close, fig)
162166

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

171171
# Note that exceptions would be printed to stderr; _isolated_tk_test
172172
# checks them.

‎lib/matplotlib/tests/test_backends_interactive.py

Copy file name to clipboardExpand all lines: lib/matplotlib/tests/test_backends_interactive.py
+56-2Lines 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()
@@ -503,3 +505,55 @@ def test_blitting_events(env):
503505
# blitting is not properly implemented
504506
ndraws = proc.stdout.count("DrawEvent")
505507
assert 0 < ndraws < 5
508+
509+
510+
# The source of this function gets extracted and run in another process, so it
511+
# must be fully self-contained.
512+
def _test_figure_leak():
513+
import gc
514+
import sys
515+
516+
import psutil
517+
from matplotlib import pyplot as plt
518+
# Second argument is pause length, but if zero we should skip pausing
519+
t = float(sys.argv[1])
520+
p = psutil.Process()
521+
522+
# Warmup cycle, this reasonably allocates a lot
523+
for _ in range(2):
524+
fig = plt.figure()
525+
if t:
526+
plt.pause(t)
527+
plt.close(fig)
528+
mem = p.memory_info().rss
529+
gc.collect()
530+
531+
for _ in range(5):
532+
fig = plt.figure()
533+
if t:
534+
plt.pause(t)
535+
plt.close(fig)
536+
gc.collect()
537+
growth = p.memory_info().rss - mem
538+
539+
print(growth)
540+
541+
542+
# TODO: "0.1" memory threshold could be reduced 10x by fixing tkagg
543+
@pytest.mark.parametrize("env", _get_testable_interactive_backends())
544+
@pytest.mark.parametrize("time_mem", [(0.0, 2_000_000), (0.1, 30_000_000)])
545+
def test_figure_leak_20490(env, time_mem):
546+
pytest.importorskip("psutil", reason="psutil needed to run this test")
547+
548+
# We haven't yet directly identified the leaks so test with a memory growth
549+
# threshold.
550+
pause_time, acceptable_memory_leakage = time_mem
551+
if env["MPLBACKEND"] == "macosx":
552+
acceptable_memory_leakage += 10_000_000
553+
554+
result = _run_helper(
555+
_test_figure_leak, str(pause_time), timeout=_test_timeout, **env
556+
)
557+
558+
growth = int(result.stdout)
559+
assert growth <= acceptable_memory_leakage

‎requirements/testing/all.txt

Copy file name to clipboardExpand all lines: requirements/testing/all.txt
+1Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.