Skip to content

Fix race condition: serialize Pango/Cairo calls for thread safety#125

Open
SoldierSacha wants to merge 2 commits intoManimCommunity:v0from
SoldierSacha:claude/investigate-race-condition-j4U0X
Open

Fix race condition: serialize Pango/Cairo calls for thread safety#125
SoldierSacha wants to merge 2 commits intoManimCommunity:v0from
SoldierSacha:claude/investigate-race-condition-j4U0X

Conversation

@SoldierSacha
Copy link

Summary

  • Adds a module-level threading.Lock to serialize all calls into Pango/Cairo
    and font registration, fixing a race condition caused by concurrent access to
    global state (the default PangoCairoFontMap, Fontconfig config, and the
    shared registered_fonts set).
  • Replaces star imports (from .cmanimpango import *, etc.) with explicit
    imports and a _synchronized decorator, making the public API surface clear
    and the thread-safety wrapping declarative.

Problem

Pango and Cairo use process-global state internally (the default font map,
Fontconfig configuration). When multiple threads call text2svg,
MarkupUtils.text2svg, or font registration functions concurrently, this
leads to corrupted state, segfaults, or incorrect rendering — particularly
in tools like Manim that render scenes in parallel.

Approach

A single threading.Lock (_pango_lock) in __init__.py wraps every
public function that touches Pango/Cairo or the font registry. A small
_synchronized decorator (using functools.wraps) keeps the code DRY
and preserves function metadata (__name__, __doc__, etc.).

Only manimpango/__init__.py is changed — no modifications to the Cython
layer or register_font.py.

The registered_fonts global set is read during text2svg rendering and
written during font registration, with no synchronization. Concurrent
calls can cause RuntimeError from set mutation during iteration.
Additionally, Pango's default font map is shared global state accessed
by all rendering calls.

Wrap text2svg, MarkupUtils.text2svg, register_font, unregister_font,
fc_register_font, fc_unregister_font, and list_fonts with a single
module-level threading.Lock to serialize access to these shared
resources.

https://claude.ai/code/session_012X3a138VZEKecNvQuoyFmm
…d decorator

Replace the star-import-then-shadow pattern with explicit imports for
all public names, and a reusable _synchronized decorator that wraps
functions with the thread-safety lock. This eliminates the try/except/else
gymnastics and makes the wrapping declarative and readable.

https://claude.ai/code/session_012X3a138VZEKecNvQuoyFmm
Copy link
Member

@naveen521kk naveen521kk left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I took a look around this, and it looks like a workaround solution instead of fixing the actual issue. Instead, we have to change the Cython implementation to implement locks, if required, accordingly.

Do you have a Minimal Reproducible Example where we can reproduce this issue? We need it to add it to the test suite.

@SoldierSacha
Copy link
Author

Thanks for your contribution. I took a look around this, and it looks like a workaround solution instead of fixing the actual issue. Instead, we have to change the Cython implementation to implement locks, if required, accordingly.

Do you have a Minimal Reproducible Example where we can reproduce this issue? We need it to add it to the test suite.

No problem! I guess you could consider it a workaround solution in that sense, since the problem does technically come from the Cython.

Unfortunately I don't have a minimal example. If that's necessary, I guess you can close the PR. I just wanted to bring this to attention.

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.

3 participants