8000 Fix TkAgg memory leaks and test for memory growth regressions by richardsheridan · Pull Request #22002 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Fix TkAgg memory leaks and test for memory growth regressions #22002

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 15 commits into from
Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ dependencies:
- nbformat!=5.0.0,!=5.0.1
- pandas!=0.25.0
- pikepdf
- psutil
- pre-commit
- pydocstyle>=5.1.0
- pytest!=4.6.0,!=5.4.0
Expand Down
16 changes: 10 additions & 6 deletions lib/matplotlib/backends/_backend_tk.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,18 +474,22 @@ def destroy(self, *args):
self.canvas._tkcanvas.after_cancel(self.canvas._event_loop_id)

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

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

# "after idle after 0" avoids Tcl error/race (GH #19940)
self.window.after_idle(self.window.after, 0, delayed_destroy)
if cbook._get_running_interactive_framework() == "tk":
# "after idle after 0" avoids Tcl error/race (GH #19940)
self.window.after_idle(self.window.after, 0, delayed_destroy)
else:
self.window.update()
delayed_destroy()

def get_window_title(self):
return self.window.wm_title()
Expand Down
14 changes: 7 additions & 7 deletions lib/matplotlib/tests/test_backend_tk.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ def test_func():
)
except subprocess.TimeoutExpired:
pytest.fail("Subprocess timed out")
except subprocess.CalledProcessError:
pytest.fail("Subprocess failed to test intended behavior")
except subprocess.CalledProcessError as e:
pytest.fail("Subprocess failed to test intended behavior\n"
+ str(e.stderr))
else:
# macOS may actually emit irrelevant errors about Accelerated
# OpenGL vs. software OpenGL, so suppress them.
Expand Down Expand Up @@ -146,6 +147,7 @@ def target():
thread.join()


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

plt.draw() # Test FigureCanvasTkAgg.
fig.canvas.toolbar.configure_subplots() # Test NavigationToolbar2Tk.
# Test FigureCanvasTk filter_destroy callback
fig.canvas.get_tk_widget().after(100, plt.close, fig)

# Check for update() or update_idletasks() in the event queue, functionally
# equivalent to tkinter.Misc.update.
# Must pause >= 1 ms to process tcl idle events plus extra time to avoid
# flaky tests on slow systems.
plt.pause(0.1)

plt.close(fig) # Test FigureCanvasTk filter_destroy callback
plt.show(block=True)

# Note that exceptions would be printed to stderr; _isolated_tk_test
# checks them.
Expand Down
46 changes: 44 additions & 2 deletions lib/matplotlib/tests/test_backends_interactive.py
8000
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ def _test_thread_impl():
future = ThreadPoolExecutor().submit(fig.canvas.draw)
plt.pause(0.5) # flush_events fails here on at least Tkagg (bpo-41176)
future.result() # Joins the thread; rethrows any exception.
plt.close()
fig.canvas.flush_events() # pause doesn't process events after close
plt.close() # backend is responsible for flushing any events here


_thread_safe_backends = _get_testable_interactive_backends()
Expand Down Expand Up @@ -503,3 +502,46 @@ def test_blitting_events(env):
# blitting is not properly implemented
ndraws = proc.stdout.count("DrawEvent")
assert 0 < ndraws < 5


# The source of this function gets extracted and run in another process, so it
# must be fully self-contained.
def _test_figure_leak():
import sys

import psutil
from matplotlib import pyplot as plt
# Second argument is pause length, but if zero we should skip pausing
t = float(sys.argv[1])

# Warmup cycle, this reasonably allocates a lot
p = psutil.Process()
fig = plt.figure()
if t:
plt.pause(t)
plt.close(fig)
mem = p.memory_full_info().uss

for _ in range(5):
fig = plt.figure()
if t:
plt.pause(t)
plt.close(fig)
growth = p.memory_full_info().uss - mem

print(growth)


@pytest.mark.parametrize("env", _get_testable_interactive_backends())
@pytest.mark.parametrize("time", ["0.0", "0.1"])
def test_figure_leak_20490(env, time):
pytest.importorskip("psutil", reason="psutil needed to run this test")

# We can't yet directly identify the leak
# so test with a memory growth threshold
acceptable_memory_leakage = 2_000_000

result = _run_helper(_test_figure_leak, time, timeout=_test_timeout, **env)

growth = int(result.stdout)
assert growth <= acceptable_memory_leakage
1 change: 1 addition & 0 deletions requirements/testing/all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

certifi
coverage<6.3
psutil
pytest!=4.6.0,!=5.4.0
pytest-cov
pytest-rerunfailures
Expand Down
0