Skip to content

Fix three issues with mo.mpl.interactive in marimo run mode (#8747):#8760

Merged
mscolnick merged 3 commits intomainfrom
ms/mpl-fixes
Mar 19, 2026
Merged

Fix three issues with mo.mpl.interactive in marimo run mode (#8747):#8760
mscolnick merged 3 commits intomainfrom
ms/mpl-fixes

Conversation

@mscolnick
Copy link
Copy Markdown
Contributor

  1. Suppress startup warning: "Starting a Matplotlib GUI outside of the main thread will likely fail" — this is a false positive since WebAgg only uses software (Agg) rendering and communicates over the network, no GUI toolkit is involved.

  2. Reduce log noise: Downgrade "Received message for unknown comm" from WARNING to DEBUG — this is an expected race condition when comms are closed during cell re-execution or server shutdown.

  3. Fix shutdown crash: Properly clean up the figure manager (remove web socket, close canvas) when a cell is re-run or deleted, preventing dangling timer callbacks that cause "QObject::killTimer: Timers cannot be stopped from another thread" errors and segfaults.

1. **Suppress startup warning**: "Starting a Matplotlib GUI outside of the main thread will likely fail" — this is a false positive since WebAgg only uses software (Agg) rendering and communicates over the network, no GUI toolkit is involved.

2. **Reduce log noise**: Downgrade "Received message for unknown comm" from WARNING to DEBUG — this is an expected race condition when comms are closed during cell re-execution or server shutdown.

3. **Fix shutdown crash**: Properly clean up the figure manager (remove web socket, close canvas) when a cell is re-run or deleted, preventing dangling timer callbacks that cause "QObject::killTimer: Timers cannot be stopped from another thread" errors and segfaults.
Copilot AI review requested due to automatic review settings March 18, 2026 16:18
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Mar 19, 2026 1:37am

Request Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves mo.mpl.interactive behavior in marimo run mode by reducing noisy/incorrect warnings, lowering expected comm-race log severity, and adding cleanup to prevent backend shutdown crashes.

Changes:

  • Suppress Matplotlib’s “GUI outside of main thread” warning when creating WebAgg figure managers.
  • Downgrade “unknown comm” messages from WARNING to DEBUG due to expected comm lifecycle races.
  • Add figure-manager + websocket cleanup during cell lifecycle disposal, with accompanying tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
marimo/_plugins/stateless/mpl/_mpl.py Suppresses a known-false-positive Matplotlib thread warning when creating WebAgg managers.
marimo/_plugins/ui/_impl/comm.py Reduces log noise by demoting “unknown comm” to DEBUG with explanatory comment.
marimo/_plugins/ui/_impl/from_mpl_interactive.py Extends cell lifecycle cleanup to detach websocket and close canvas before closing the comm.
tests/_plugins/ui/_impl/test_from_mpl_interactive.py Adds tests asserting figure-manager cleanup behavior and error tolerance.
tests/_plugins/stateless/test_mpl.py Adds regression test ensuring no thread warning is emitted during manager creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


class _MplCleanupHandle(CellLifecycleItem):
"""Closes the MarimoComm when the cell is re-run or deleted."""
"""Cleans up the matplotlib figure manager and MarimoComm on cell re-run."""
Comment on lines 317 to 333
def dispose(self, context: RuntimeContext, deletion: bool) -> bool:
del context, deletion
# Disconnect the web socket from the figure manager first,
# then destroy the canvas to prevent stale timer callbacks
# on thread teardown (avoids "QObject::killTimer" errors).
if self._figure_manager is not None and self._sync_ws is not None:
try:
self._figure_manager.remove_web_socket(self._sync_ws)
except Exception:
pass
if self._figure_manager is not None:
try:
self._figure_manager.canvas.close()
except Exception:
pass
self._comm.close()
return True
Comment on lines +175 to +176
new_figure_manager_given_figure(id(fig), fig)
captured.extend(w)
Copy link
Copy Markdown
Collaborator

@dmadisetti dmadisetti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the test is just brittle

tests/_plugins/ui/_impl/test_comm.py::test_comm_manager_receive_unknown_message

- Update test_comm_manager_receive_unknown_message to assert debug (not warning)
- Update _MplCleanupHandle docstring to mention both re-run and deletion
- Clean up figure manager in thread warning test to prevent resource leaks
@mscolnick mscolnick merged commit fa67e1d into main Mar 19, 2026
42 of 43 checks passed
@mscolnick mscolnick deleted the ms/mpl-fixes branch March 19, 2026 14:51
@mscolnick mscolnick mentioned this pull request Mar 20, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants