MigraDoc FontHandler's single-entry font cache is racy: concurrent rendering silently returns the wrong font

[tgburgin]

Summary

MigraDoc.Rendering.FontHandler.FontToXFont memoises the last (Font, XFont) pair in two independent static fields, written with no synchronisation. Under concurrent rendering a thread can be handed an XFont belonging to a different Font — in practice, text drawn or measured at another document's font size.

Nothing throws. The PDF is produced successfully and is silently typeset wrong.

The code (master, and v7.0.0-preview-1)

src/foundation/src/MigraDoc/src/MigraDoc.Rendering/Rendering/FontHandler.cs

internal static XFont FontToXFont(Font font)
{
    // Check if both WeakReferences are still valid and point to the font we need.
    if (_lastXFont != null && _lastFont != null &&
        _lastFont.TryGetTarget(out var lastFont) && font == lastFont &&
        _lastXFont.TryGetTarget(out var lastXFont))
        return lastXFont;
    ...
    _lastFont = new(font);
    _lastXFont = new(xFont);
    return xFont;
}

static WeakReference<XFont>? _lastXFont;
static WeakReference<Font>? _lastFont;

The interleaving

	thread A (font = 9 pt)	thread B (font = 6.5 pt)
1	_lastFont.TryGetTarget(...) → A's font, font == lastFont ✔
2		_lastFont = new(fontB); _lastXFont = new(xFontB);
3	_lastXFont.TryGetTarget(out lastXFont) → B's XFont
4	return lastXFont; → 6.5 pt returned for a 9 pt Font

Each _lastXFont in the condition is a separate field read, so the guard proves nothing about the value eventually returned.

Every layout and every draw in MigraDoc goes through this method, so the corruption can land on measurement (wrong string widths → wrong wrapping and wrong line breaks) or on drawing (a Tf at the wrong size mid-line).

Reproduction

A. Directly, against FontToXFont. internal, so via reflection. Two stable Font instances are essential — the memo is keyed on the object, so allocating a fresh Font per call never hits the cache and the race is invisible.

GlobalFontSettings.FontResolver ??= new MyFontResolver();

var m = typeof(DocumentRenderer).Assembly.GetType("MigraDoc.Rendering.FontHandler")!
    .GetMethod("FontToXFont", BindingFlags.Static | BindingFlags.NonPublic)!;

var fonts = new[] { new Font("Arial") { Size = 9.0 }, new Font("Arial") { Size = 6.5 } };
var tears = 0;

Parallel.For(0, 200_000, new ParallelOptions { MaxDegreeOfParallelism = 8 }, i =>
{
    var font = fonts[i % 2];
    var got = (XFont)m.Invoke(null, new object?[] { font })!;
    if (Math.Abs(got.Size - font.Size.Point) > 0.001)
        Interlocked.Increment(ref tears);
});

Console.WriteLine($"{tears} tears");

Observed on .NET 10 / Windows, 8 threads: 8 tears in 200,000 calls, both directions —
asked 6.5pt, returned 9pt and asked 9pt, returned 6.5pt.

Isolating just the cache shape (so XFont construction no longer dominates the loop) gives 104,457 tears in 400,000 when two Font instances are equally hot. Restructured as in the linked PR: 0.

B. End-to-end, as a user sees it. Render one document repeatedly and compare the decompressed page content streams — the PDF bytes themselves always differ because of the embedded creation date, but the content streams should be identical for identical input:

• sequential, 300 renders → 1 distinct content stream
• Parallel.For, 8 threads, 300 renders → 34–49 distinct content streams

A captured divergence was a single word's advance changing by 0.2778 pt, with the line's carriage-return offset shifted by exactly the same amount — one word measured with the wrong font. In another capture a footer line wrapped differently and a word moved to its own line.

Why this one is easy to miss

PDFsharp's own font caches are synchronised — Locks.EnterFontManagement() in PdfSharp.Internal.Threading.Locks, complete with the debug counters and the lock-order-inversion guard against the GDI+ lock. Probing XGraphics.MeasureString directly over the same words and sizes at 8 threads × 400 iterations returns a single value per pair; it is solid.

FontHandler sits one layer above that and takes no lock at all, so it is the one unguarded step in an otherwise guarded font path.

Affected versions

• 6.2.0 — same bug, plain fields: if (_lastXFont != null && font == _lastFont) return _lastXFont;
• 6.2.1 – 6.2.4 — the fields became WeakReference<T> (a memory-leak fix, cf. the FORCE_MEMORYLEAK conditional beside them); the race is untouched
• master / v7.0.0-preview-1 — unchanged; the file carries a // v7.0.0 REVIEW PSG marker

I could not find an existing report — I scanned all 313 issues and PRs, open and closed. #330 is a performance refactor of the PdfSharp caches (already correctly locked) and does not touch this file.

Suggested fix

Publish both references as one immutable entry assigned to a single volatile field, so a reader sees either the complete previous entry or the complete new one. The cache-hit fast path, the allocation count and the behaviour are otherwise unchanged. PR follows.

Environment

.NET 10, Windows 11 x64, PDFsharp-MigraDoc 6.2.0, custom IFontResolver.