Skip to content

Fix data race in MigraDoc FontHandler's single-entry font cache #382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tgburgin wants to merge 1 commit into
base: master
Choose a base branch
from
+26 −8
Open

Fix data race in MigraDoc FontHandler's single-entry font cache #382

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions src/foundation/src/MigraDoc/src/MigraDoc.Rendering/Rendering/FontHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ static class FontHandler
/// </summary>
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))
// Read the whole cache entry once. The Font and the XFont are published together by a single
// reference assignment, so a concurrent update can never be observed half-applied and this can
// never return an XFont belonging to a different Font. See FontCacheEntry.
var entry = _lastEntry;
if (entry != null &&
entry.FontRef.TryGetTarget(out var lastFont) && font == lastFont &&
entry.XFontRef.TryGetTarget(out var lastXFont))
return lastXFont;

XFontStyleEx style = GetXStyle(font);
Expand All @@ -39,16 +42,31 @@ internal static XFont FontToXFont(Font font)
#if DEBUG_
CreateFontCounter++;
#endif
_lastFont = new(font);
_lastXFont = new(xFont);
_lastEntry = new FontCacheEntry(font, xFont);
#if FORCE_MEMORYLEAK
_lastFont2 = font;
#endif
return xFont;
}

static WeakReference<XFont>? _lastXFont;
static WeakReference<Font>? _lastFont;
/// <summary>
/// The single-entry font cache. Both WeakReferences are assigned in the constructor and the instance
/// is published by one atomic reference assignment, so a reader sees either the complete previous
/// entry or the complete new one — never the Font from one and the XFont from another.
/// </summary>
sealed class FontCacheEntry
{
internal FontCacheEntry(Font font, XFont xFont)
{
FontRef = new WeakReference<Font>(font);
XFontRef = new WeakReference<XFont>(xFont);
}

internal readonly WeakReference<Font> FontRef;
internal readonly WeakReference<XFont> XFontRef;
}

static volatile FontCacheEntry? _lastEntry;
#if FORCE_MEMORYLEAK
static Font? _lastFont2;
#endif
Expand Down