fix(FUN-4): prevent infinite loop in glyph assembly on degenerate extenders

[kostub]

Summary

Fixes FUN-4 (issues.md#L213).

The loop for (int numExtenders = 0; true; numExtenders++) in
-[MTTypesetter constructGlyphWithParts:height:glyphs:offsets:height:]
only exits when the assembled minHeight (branch A) or maxHeight (branch B)
reaches the requested glyphHeight. A font plist that supplies an extender
part with fullAdvance == 0 gives minOffsetDelta <= 0 each iteration:
minHeight never grows, maxHeight stalls or shrinks, and the loop spins
forever on the UI thread — hanging the app.

Fix (1 file, ~18 LOC, no API change)

Two-layer guard inside constructGlyphWithParts::

1. No-progress guard — after both normal exit branches fail, if neither
   minHeight nor maxHeight grew by >= 0.01 pt vs the previous iteration,
   return the best-effort assembly rather than looping. Both heights must stall
   to trigger the guard; checking only minHeight would falsely fire on valid
   fonts (e.g. arrowright in Latin Modern Math) where extender-to-extender
   joints contribute zero to minOffset but maxHeight grows normally until
   branch B fires.

2. Hard iteration cap (numExtenders <= 10000) as belt-and-suspenders
   against any degenerate arithmetic the progress check might miss.

Valid fonts are byte-for-byte unaffected.

Tests

New MTGlyphAssemblyTest (3 tests, all green):

• testConstructGlyphWithZeroAdvanceExtenderTerminates — synthetic parts
  with a zero-advance extender flanked by fixed parts (the exact hang
  configuration); asserts the method returns a non-empty best-effort
  assembly. Without the fix this hangs and is caught by the XCTest timeout.
• testConstructGlyphWithPositiveAdvanceExtenderProducesAdequateHeight —
  normal positive-advance extender reaches the requested height via branch A.
• testTallDelimiterRendersWithBundledFont — end-to-end regression guard:
  a tall \left( \frac{\frac{1}{2}}{\frac{3}{4}} \right) renders with
  positive dimensions using a bundled font (no behaviour change on valid data).

Two test-only headers (MTGlyphPart+Testing.h, MTTypesetter+Testing.h)
expose private seams to the test target; they are not part of the public API.

Test plan

• swift build — clean
• swift test — 295 tests, 0 failures

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request implements a loop-termination guard in MTTypesetter.m to prevent infinite loops on the UI thread when handling malformed font MATH data with zero-advance extenders, supported by new unit tests. The review feedback identifies a high-severity memory accumulation risk where running up to 10,000 iterations can allocate millions of autoreleased objects, and suggests reducing the iteration cap to 1,000 and using an @autoreleasepool block to mitigate potential out-of-memory crashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Memory Accumulation Risk on High Iteration Counts

In the worst-case scenario (e.g., if a font has a very small but positive advance, or if an extremely large glyphHeight is requested), the loop can run up to kMaxExtenders times.

Because the loop builds the entire glyph and offset arrays from scratch in each iteration, the number of allocations grows quadratically:
$$\sum_{i=1}^{N} 2i = N(N+1)$$
For $N = 10000$, this results in over 100 million autoreleased objects (NSNumber and NSMutableArray instances) being accumulated on the UI thread's autorelease pool. This will cause a massive memory spike and likely trigger an Out-Of-Memory (OOM) crash or a severe UI freeze.

Recommendations:

1. Reduce kMaxExtenders: A limit of 1000 is still extremely generous (equivalent to several times the height of any iPad screen) but limits the maximum allocations to a safe level (~1 million objects in the absolute worst case).
2. Use @autoreleasepool: Wrap the body of the for loop in an @autoreleasepool block to ensure that temporary arrays and numbers from previous iterations are drained immediately rather than accumulating.

    static const int kMaxExtenders = 1000;

[kostub]

EM-REVIEW v1

Code review: FUN-4 glyph-assembly infinite-loop guard. Reviewed head cfc939b.

Severity: BLOCKING (1 blocking issue).

BLOCKING - New iOS test not wired into iosMath.xcodeproj, so it never runs in CI.
iosMathTests/MTGlyphAssemblyTest.m is absent from iosMath.xcodeproj/project.pbxproj. This project uses the legacy explicit-file model (not a PBXFileSystemSynchronizedRootGroup): every test .m is listed in PBXFileReference, PBXBuildFile, the iosMathTests group, and the iosMathTests Sources build phase (e.g. MTTypesetterTest.m, MTMathListBuilderTest.m). The new file is in none of them. CI runs the suite two ways: swift test (SPM, directory-globbed, picks the file up) and xcodebuild test -project iosMath.xcodeproj -scheme iosMath on the iOS simulator (ci.yml lines 63-70). The latter compiles only files enumerated in the target, so all three new tests - including testConstructGlyphWithZeroAdvanceExtenderTerminates, the hang regression guard - silently never execute on the iOS path. The 295-tests/0-failures result reflects only the SPM run. The fix itself is in MTTypesetter.m (library target) so it ships, but iOS test coverage is missing. Fix: add MTGlyphAssemblyTest.m to the four iosMath.xcodeproj locations. The two +Testing.h headers do NOT need build-phase entries (headers resolve via include paths). MacOSMath.xcodeproj has no test target (demo app only), so nothing needed there - not a gap.

Non-blocking:

1. (Low) Hard-cap fallback can leave *glyphs/*offsets uninitialized if the loop reaches kMaxExtenders without ever completing an iteration past 'if (!prev) continue;' (lastGlyphs stays nil); callers then dereference glyphs[0] with no nil check. Unreachable with real data (no-progress guard fires first); theoretical. Not a new regression. Initializing out-params at entry or asserting lastGlyphs != nil would close it.
2. (Info) No-progress condition is correct: requiring BOTH minHeight and maxHeight to stall is right - a valid extender whose joints contribute 0 to minOffset keeps minHeight flat while maxHeight grows until branch B fires. numExtenders > 0 correctly skips the degenerate first comparison vs -CGFLOAT_MAX. 0.01pt epsilon safely below any real extender advance.
3. (Info) *height = lastMinHeight on the guard path is the correct best-effort value (branch B never reached).
4. (Info) MTGlyphPart (Testing) redeclares readonly props as (nonatomic) writable; the real class extension in MTFontMathTable.m already declares them (nonatomic) readwrite, so setters back the category and attributes match. Sound.

No regression risk to the normal path: well-formed fonts hit branch A or B as before; new code runs only after both branches fail, which valid data never reaches.

Required before merge: add MTGlyphAssemblyTest.m to iosMath.xcodeproj (PBXFileReference + PBXBuildFile + iosMathTests group + Sources build phase) and confirm xcodebuild test -project iosMath.xcodeproj -scheme iosMath runs the 3 new tests.

Not approving.

[kostub]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces validation for glyph assemblies to prevent infinite loops in the renderer caused by extender parts with non-positive advance values. It adds a validation check in the Python plist generator script (math_table_to_plist.py), implements a corresponding runtime check in the Objective-C MTFontMathTable class, and includes comprehensive unit tests to verify this behavior. There are no review comments, so no additional feedback is provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.