[Bug]: Umbrella issue: matplotlib C/C++ Extension Analysis Report #31424

Activity

1. github-actions

   added

   first-contribution

   on Mar 29, 2026
2. scottshambaugh commented on Mar 30, 2026

   

   scottshambaugh

   on Mar 30, 2026

   Contributor

   More actions

   Paraphrasing my comment from discourse:

   Thanks for running this!

   It looks like most of these are low impact but also quick to implement, and a single PR could target several in a category at once. I don't see anything urgent to address at the moment, but the discussion here will help with visibility and keeping track of the conversation. We can always pick-and-choose as needed.

   Some are not worth doing - my take is that that OOM / pybind11 issues are fundamental architecture limitations and not really something for us to address, the GIL release items are potentially tricky to put boundaries around and of limited use for this as a primarily single-threaded tool, and the free-threading concerns are premature since we need to support legacy python versions for quite a while.

   I think we on the maintainers team all knew automated code auditing tools like this were coming, but haven't figured out how we're going to handle them / how much stock to put in them. This is one of the better executed audits I've seen so far, and I think will be a useful first case for us to talk through some of those considerations.
3. melissawm

   added

   Maintenance

   on Mar 31, 2026
4. tacaswell commented on Apr 2, 2026

   

   tacaswell

   on Apr 2, 2026

   Member

   More actions

   Skimming this, I think one PR to handle the macos.m issues all together would make sense.

   I suspect that the freetype issues are going to be at least different very soon once the font refactor PR lands.

   Adding null checks is non-controversial and I don't see problems with those coming in in batches.
5. kx7m2qd commented on May 4, 2026

   

   kx7m2qd

   on May 4, 2026

   More actions

   I've opened a PR to address the PyErr_Fetch/PyErr_Restore deprecation item
   from this list: #[your PR number]

   The fix updates close_file_callback in src/ft2font_wrapper.cpp to use
   PyErr_GetRaisedException/PyErr_SetRaisedException on Python 3.12+, guarded
   by #if PY_VERSION_HEX >= 0x030c0000 for backward compatibility.
6. kx7m2qd

   added 2 commits that reference this issue on May 5, 2026

   Fix deprecated PyErr_Fetch/PyErr_Restore in close_file_callback

   ...

   94738e5

   Fix deprecated PyErr_Fetch/PyErr_Restore in close_file_callback

   ...

   c1232d8
7. clin1234 commented on Jul 19, 2026

   

   clin1234

   on Jul 19, 2026

   Last edited by clin1234

   More actions

   Updated report, as of d7bc494:

   Findings by Priority

   Global, non-restarting numbering. "Agents" notes cross-validation.

   Must Fix (FIX) — 2

   #	Finding	File:Line	Agents
   1	PyFT2Font_init is a pybind11 factory that news a PyFT2Font (line 521) but can throw before returning it, so pybind11 never adopts the pointer and it leaks — on every failed construction. Reachable triggers: io.open fails on a bad path (:546), non-binary file-like arg (:556), or FT_Open_Face rejects a corrupt/truncated/unsupported font (:563 via FT_CHECK). On the :563 path an already-open file object (py_file) is stranded too (memory always; fd on the invalid-font path). Fix: hold it in std::make_unique<PyFT2Font>(...) and return self.release(); only on success (unwind is then clean — ~PyFT2Font→close() no-ops on face==nullptr).	ft2font_wrapper.cpp:521-563 (registered :1589)	type-slot-checker + resource-lifecycle-checker (independent cross-validation)
   2	FT2Image constructor calls calloc(width * height, 1) with no NULL check, and width * height (both unsigned long) can integer-overflow → under-allocation then out-of-bounds writes in draw_bitmap/draw_rect_filled, or a NULL-deref on allocation failure. Fix: overflow-check the product and verify the result, throwing instead of returning a NULL buffer.	ft2font.cpp:21-24 (writes :50-64, :79-83)	null-safety (manual — scanner missed the C++ member-init calloc)

   Should Consider (CONSIDER) — 11

   #	Finding	File:Line
   3	Shared global FreeType library _ft2Library (non-static, so the module-state scanner missed it): every FT_Open_Face/FT_Done_Face mutates it unlocked → a data race under mod_gil_not_used() and a subinterpreter blocker. Full analysis + fix in FREETHREADING_ANALYSIS.md (Finding 1, CRITICAL).	ft2font.cpp:19, 200, 223
   4	RendererAgg constructor leaks pixBuffer (and alphaBuffer) if a later new[] throws (bad_alloc, reachable via a pathological large dpi → hatch_size²). A throwing ctor means ~RendererAgg never runs. Fix: hold the buffers in std::vector/unique_ptr. (OOM-only → LOW.)	_backend_agg.cpp:44, 52, 66
   5	FT_Glyph handle leak between FT_Get_Glyph and glyphs.push_back — if the push_back (or an earlier xys.push_back) throws bad_alloc, the in-flight thisGlyph is never FT_Done_Glyph'd. Fix: RAII-guard thisGlyph, or glyphs.reserve() before the loop. (OOM-only → LOW.)	ft2font.cpp:482→506, 558→559
   6	Reference cycles through Python objects stored as C++ members are invisible to the cyclic GC. pybind11 installs no tp_traverse/tp_clear for C++-member py::objects, so a cycle through FT2Font.py_file/fallbacks/image (or the persistent numpy arrays in _tri types) leaks the whole cycle. Lifetime itself is safe (the py::list fallbacks owns the children; the raw FT2Font*/char_to_font are borrowed views — no use-after-free). Inherent pybind11 limitation (pybind/pybind11#1029).	ft2font_wrapper.cpp:359-361; ft2font.h:185,189-190; tri/_tri.h:306-316
   7	.pyi stub drift. (a) Wrong types that break type-checkers/keyword use: _path.count_bboxes_overlapping_bbox is typed Sequence[BboxBase] but the C++ takes an ndarray (real caller passes np.atleast_3d(...)); _tri.Triangulation.__init__ param correct_triangle_orientation vs C++ ...orientations; FT2Font.draw_glyph(s)_to_bitmap/_layout/load_glyph have kw-only / renamed params vs the stub; Win32 HWND capsules typed as int. (b) Empty/partial stubs (ship despite py.typed): _qhull.pyi, _image.pyi, backends/_backend_agg.pyi, backends/_macosx.pyi are 0 bytes; _path.pyi types only 2 of 14 functions. Runtime-safe, but degrades the advertised typed API.	_path.pyi:8; _tri.pyi:31; ft2font.pyi:245-291; _qhull.pyi/_image.pyi/backends/_backend_agg.pyi/backends/_macosx.pyi (empty)
   8	Write-once static mutable state (subinterpreter blockers). _tkagg caches Tk/Tcl entry points in file-scope statics via dlsym; ft2font has a static FT_Outline_Funcs table. Safe for free-threading (write-once) but process-global — must move to per-module state for PEP 489 / subinterpreter support.	_tkagg.cpp:90,91,94,95; ft2font.cpp:149
   9	Single-phase init across all modules — combined with the globals above, blocks subinterpreter support.	all _wrapper.cpp PYBIND11_MODULE lines
   10	QhullInfo manages resources (qhT*, FILE*) but declares no copy-constructor / operator= (Rule of Three). Never copied today, so no live bug, but an accidental copy would double-fclose/double-free. Recommend = delete on both. (Destructor itself is correct — see Strengths.)	_qhull_wrapper.cpp:112-136
   11	appid CoTaskMemFree leak if py::cast throws before the free (Windows-only, one-shot, OOM). Move the free ahead of the cast or use a scope guard.	_c_internal_utils.cpp:108-109
   12	LayoutItem::ft_object is a borrowed PyFT2Font* exposed via def_readonly with no keep-alive — a LayoutItem outliving its font would dangle. Transient, private (_layout) API → safe today; guard only if LayoutItems are ever stored.	ft2font_wrapper.cpp:1382, 1566
   13	mod_gil_not_used() asserted before it is enforced — no std::mutex/py::scoped_critical_section anywhere in src/. Until Finding 3 is guarded, the thread-safety claim on all 8 modules is unsound.	project-wide

   Tensions

   • Subinterpreters vs. simplicity (Findings 8–9): module-state-checker flags single-phase init and process-global state, but the codebase is small and low-complexity and there is no stated subinterpreter goal. Free-threading (the active effort) does not require multi-phase init — only synchronization of the shared state.
   • mod_gil_not_used() vs. current reality (Finding 13): the C-level thread-safety claim is ahead of the C-level guarantees; the Python-layer thread-keyed font cache only partially compensates.
   • Ownership vs. GC-visibility (Finding 6): the double bookkeeping of fallbacks (owning py::list + borrowed FT2Font*) is correct for lifetime but is exactly what makes cycles GC-invisible; fixing GC-visibility would mean custom type slots, adding complexity this otherwise-clean code avoids.

   Source: https://gist.github.com/clin1234/2469758160721f4f991110167017e4d3#file-matplotlib-md