Skip to content

Make Tkagg blit thread safe #18565

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 19 commits into from
Nov 2, 2020
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
68 changes: 61 additions & 7 deletions lib/matplotlib/backends/_backend_tk.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import uuid
from contextlib import contextmanager
import logging
import math
Expand Down Expand Up @@ -44,6 +45,28 @@ def _restore_foreground_window_at_end():
_c_internal_utils.Win32_SetForegroundWindow(foreground)


_blit_args = {}
# Initialize to a non-empty string that is not a Tcl command
_blit_tcl_name = "mpl_blit_" + uuid.uuid4().hex


def _blit(argsid):
"""
Thin wrapper to blit called via tkapp.call.

*argsid* is a unique string identifier to fetch the correct arguments from
the ``_blit_args`` dict, since arguments cannot be passed directly.

photoimage blanking must occur in the same event and thread as blitting
to avoid flickering.
"""
photoimage, dataptr, offsets, bboxptr, blank = _blit_args.pop(argsid)
if blank:
photoimage.blank()
_tkagg.blit(
photoimage.tk.interpaddr(), str(photoimage), dataptr, offsets, bboxptr)


def blit(photoimage, aggimage, offsets, bbox=None):
"""
Blit *aggimage* to *photoimage*.
Expand All @@ -53,7 +76,10 @@ def blit(photoimage, aggimage, offsets, bbox=None):
(2, 1, 0, 3) for little-endian ARBG32 (i.e. GBRA8888) data and (1, 2, 3, 0)
for big-endian ARGB32 (i.e. ARGB8888) data.

If *bbox* is passed, it defines the region that gets blitted.
If *bbox* is passed, it defines the region that gets blitted. That region
will NOT be blanked before blitting.

Tcl events must be dispatched to trigger a blit from a non-Tcl thread.
"""
data = np.asarray(aggimage)
height, width = data.shape[:2]
Expand All @@ -65,11 +91,31 @@ def blit(photoimage, aggimage, offsets, bbox=None):
y1 = max(math.floor(y1), 0)
y2 = min(math.ceil(y2), height)
bboxptr = (x1, x2, y1, y2)
blank = False
else:
photoimage.blank()
bboxptr = (0, width, 0, height)
_tkagg.blit(
photoimage.tk.interpaddr(), str(photoimage), dataptr, offsets, bboxptr)
blank = True

# NOTE: _tkagg.blit is thread unsafe and will crash the process if called
# from a thread (GH#13293). Instead of blanking and blitting here,
# use tkapp.call to post a cross-thread event if this function is called
# from a non-Tcl thread.

# tkapp.call coerces all arguments to strings, so to avoid string parsing
# within _blit, pack up the arguments into a global data structure.
args = photoimage, dataptr, offsets, bboxptr, blank
# Need a unique key to avoid thread races.
# Again, make the key a string to avoid string parsing in _blit.
argsid = str(id(args))
_blit_args[argsid] = args

try:
photoimage.tk.call(_blit_tcl_name, argsid)
except tk.TclError as e:
if "invalid command name" not in str(e):
raise
photoimage.tk.createcommand(_blit_tcl_name, _blit)
photoimage.tk.call(_blit_tcl_name, argsid)


class TimerTk(TimerBase):
Expand Down Expand Up @@ -402,10 +448,18 @@ def destroy(self, *args):
if self.canvas._idle_callback:
self.canvas._tkcanvas.after_cancel(self.canvas._idle_callback)

self.window.destroy()
# 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.
def delayed_destroy():
self.window.destroy()

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

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

def get_window_title(self):
return self.window.wm_title()
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/backends/backend_tkagg.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
class FigureCanvasTkAgg(FigureCanvasAgg, FigureCanvasTk):
def draw(self):
super().draw()
_backend_tk.blit(self._tkphoto, self.renderer._renderer, (0, 1, 2, 3))
self.blit()

def blit(self, bbox=None):
_backend_tk.blit(
Expand Down
85 changes: 84 additions & 1 deletion lib/matplotlib/tests/test_backends_interactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ def _get_testable_interactive_backends():
# early. Also, gtk3 redefines key_press_event with a different signature, so
# we directly invoke it from the superclass instead.
_test_script = """\
import importlib
import importlib.util
import io
import json
Expand Down Expand Up @@ -173,6 +172,90 @@ def test_interactive_backend(backend, toolbar):
assert proc.stdout.count("CloseEvent") == 1


_thread_test_script = """\
import json
import sys
import threading

from matplotlib import pyplot as plt, rcParams
rcParams.update({
"webagg.open_in_browser": False,
"webagg.port_retries": 1,
})
if len(sys.argv) >= 2: # Second argument is json-encoded rcParams.
rcParams.update(json.loads(sys.argv[1]))

# Test artist creation and drawing does not crash from thread
# No other guarantees!
fig, ax = plt.subplots()
# plt.pause needed vs plt.show(block=False) at least on toolbar2-tkagg
plt.pause(0.5)

exc_info = None

def thread_artist_work():
try:
ax.plot([1,3,6])
except:
# Propagate error to main thread
import sys
global exc_info
exc_info = sys.exc_info()

def thread_draw_work():
try:
fig.canvas.draw()
except:
# Propagate error to main thread
import sys
global exc_info
exc_info = sys.exc_info()

t = threading.Thread(target=thread_artist_work)
t.start()
# artists never wait for the event loop to run, so just join
t.join()

if exc_info: # Raise thread error
raise exc_info[1].with_traceback(exc_info[2])

t = threading.Thread(target=thread_draw_work)
fig.canvas.mpl_connect("close_event", print)
t.start()
plt.pause(0.5) # flush_events fails here on at least Tkagg (bpo-41176)
t.join()
plt.close()
fig.canvas.flush_events() # pause doesn't process events after close

if exc_info: # Raise thread error
raise exc_info[1].with_traceback(exc_info[2])
"""

_thread_safe_backends = _get_testable_interactive_backends()
# Known unsafe backends. Remove the xfails if they start to pass!
if "wx" in _thread_safe_backends:
_thread_safe_backends.remove("wx")
_thread_safe_backends.append(
pytest.param("wx", marks=pytest.mark.xfail(
raises=subprocess.CalledProcessError, strict=True)))
if "macosx" in _thread_safe_backends:
_thread_safe_backends.remove("macosx")
_thread_safe_backends.append(
pytest.param("macosx", marks=pytest.mark.xfail(
raises=subprocess.TimeoutExpired, strict=True)))


@pytest.mark.parametrize("backend", _thread_safe_backends)
@pytest.mark.flaky(reruns=3)
def test_interactive_thread_safety(backend):
proc = subprocess.run(
[sys.executable, "-c", _thread_test_script],
env={**os.environ, "MPLBACKEND": backend, "SOURCE_DATE_EPOCH": "0"},
timeout=_test_timeout, check=True,
stdout=subprocess.PIPE, universal_newlines=True)
assert proc.stdout.count("CloseEvent") == 1


@pytest.mark.skipif('TF_BUILD' in os.environ,
reason="this test fails an azure for unknown reasons")
@pytest.mark.skipif(os.name == "nt", reason="Cannot send SIGINT on Windows.")
Expand Down