Skip to content

monkeypatch _mathtext.Parser.parse() with a threading.Lock() - #598

Draft
nvaytet wants to merge 3 commits into
mainfrom
monkeypatch-math-parser
Draft

monkeypatch _mathtext.Parser.parse() with a threading.Lock()#598
nvaytet wants to merge 3 commits into
mainfrom
monkeypatch-math-parser

Conversation

@nvaytet

@nvaytet nvaytet commented Jul 31, 2026

Copy link
Copy Markdown
Member

Copied from matplotlib/ipympl#621

I don't know if this is the right place to put it, with the lazy_loader and everything. We may not want to do that work as soon as we import plopp?

I think we could potentially move it into the backends/matplotlib/__init__.py because the other places where matplotlib is used (e.g. making the colorbar for 3d scatter plots) don't make use of ipympl.

Either choice would lead to a somewhat possibly surprising behaviour for other plain MPL figures:

  • if we keep it in top-level init, after importing plopp, it would fix other custom mpl figures
  • in the backends/matplotlib/init, it would fix other figures after we have made a 1d or 2d figure with plopp

@nvaytet
nvaytet requested a review from SimonHeybrock July 31, 2026 10:23
@SimonHeybrock

SimonHeybrock commented Aug 3, 2026

Copy link
Copy Markdown
Member

Good find, I did not remember that PR.

I had Claude check that the lock genuinely does what it claims, and discussed whether we still need #597 (concluding to reduce it to docs-only)


(Claude's writing below): [...] four threads through _mathtext.Parser.parse with cache-defeating strings segfaults reliably here, and is clean with the lock in place. So this isn't a "does it work" objection.

Two things still make me want to skip it on our side.

It landed upstream: ipympl#621 was merged on 31 July, presumably after your ping. Latest release is still 0.10.0, so there is a real gap, but carrying a copy means double-locking once ipympl cuts a release, and remembering to remove it.

More importantly, the parser lock is only the first step of a chain. The comment on ipympl#610 serialized successive chokepoints with a shared RLock and watched the failure move each time:

  • locking the parser removes the ValueError, and the race resurfaces as blank canvases and hard kernel crashes
  • additionally locking FigureCanvasAgg.draw resurfaces RuntimeError: ... did not call Figure.draw
  • additionally locking print_figure still leaves the frame path uncovered, since get_diff_image() reads renderer.buffer_rgba() and handle_resize mutates the figure on the comm thread, with no stable API to lock

On top of that, lock contention itself can starve the comm thread, so canvases stay blank until execution finishes anyway. I have not reproduced those downstream steps myself, only the parser race.

That is what tips it for me: we would trade a loud traceback for silent blank figures, which is harder to diagnose rather than easier.

On your placement question, the top-level __init__ also costs us the lazy loading. import plopp is ~35 ms today and imports no matplotlib at all; pulling in matplotlib._mathtext drags font_manager along and takes it to ~200 ms.

I have reduced #597 to documentation only, pointing at commsOverSubshells: disabled and ipykernel<7, since those are the only workarounds reported as fully stable. Happy to be wrong here: if you can get a stable canvas with the lock under JupyterLab >= 4.4 and ipykernel >= 7, that would be worth reporting on ipympl#610.

@nvaytet

nvaytet commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Maybe we should wait for ipympl to be released?

@nvaytet
nvaytet marked this pull request as draft August 3, 2026 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants