diff --git a/.gitignore b/.gitignore index 7b9a7c5a25..8272eec0da 100644 --- a/.gitignore +++ b/.gitignore @@ -221,3 +221,6 @@ src/Browser/Avalonia.Browser/wwwroot api/diff src/Browser/Avalonia.Browser/staticwebassets .serena + +# Claude agent worktrees +.claude/worktrees/ diff --git a/src/Avalonia.Base/Media/FontManager.cs b/src/Avalonia.Base/Media/FontManager.cs index 1f15820b9a..60200dea85 100644 --- a/src/Avalonia.Base/Media/FontManager.cs +++ b/src/Avalonia.Base/Media/FontManager.cs @@ -351,36 +351,58 @@ namespace Avalonia.Media return []; } - private bool TryGetFontCollection(Uri source, [NotNullWhen(true)] out IFontCollection? fontCollection) + internal bool TryGetFontCollection(Uri source, [NotNullWhen(true)] out IFontCollection? fontCollection) { Debug.Assert(source.IsAbsoluteUri); - if (source.Scheme == SystemFontScheme) + // Both the systemfont: scheme and SystemFontsKey (fonts:SystemFonts) map to the system + // font collection. SystemFontsKey is checked before the generic IsFontCollection branch + // so that the SystemFontCollection is created on demand regardless of which URI form is used. + if (source.Scheme == SystemFontScheme || source == SystemFontsKey) { - source = SystemFontsKey; + fontCollection = GetOrCreateFontCollection(SystemFontsKey, PlatformImpl, + static (_, impl) => new SystemFontCollection(impl)); + return true; } - if (!_fontCollections.TryGetValue(source, out fontCollection)) + // Other fonts: URIs are only returned when they have been explicitly registered + // via AddFontCollection — no implicit creation to avoid caching null for unknown keys. + if (source.IsFontCollection()) { - if (source == SystemFontsKey) - { - fontCollection = new SystemFontCollection(PlatformImpl); - } - else - { - if (source.IsAbsoluteResm() || source.IsAvares()) - { - fontCollection = new EmbeddedFontCollection(source, source); - } - } + return _fontCollections.TryGetValue(source, out fontCollection); + } - if (fontCollection != null) - { - return _fontCollections.TryAdd(fontCollection.Key, fontCollection); - } + if (source.IsAbsoluteResm() || source.IsAvares()) + { + fontCollection = GetOrCreateFontCollection(source, 0, + static (key, _) => new EmbeddedFontCollection(key, key)); + return true; } - return fontCollection != null; + fontCollection = null; + return false; + } + + /// + /// Thread-safe get-or-create that disposes any candidate that loses the insertion race, + /// preventing resource leaks that + /// can cause when the factory is invoked concurrently by multiple threads. + /// + private IFontCollection GetOrCreateFontCollection(Uri key, TState state, Func factory) + { + if (_fontCollections.TryGetValue(key, out var existing)) + return existing; + + var candidate = factory(key, state); + + // GetOrAdd(key, value) atomically inserts or returns the existing value; + // it never invokes a factory, so only one IFontCollection instance survives. + var winner = _fontCollections.GetOrAdd(key, candidate); + + if (!ReferenceEquals(winner, candidate)) + candidate.Dispose(); // Our candidate lost the race – dispose it to avoid the leak. + + return winner; } private string GetDefaultFontFamilyName(FontManagerOptions? options) diff --git a/tests/Avalonia.Base.UnitTests/Media/FontManagerTests.cs b/tests/Avalonia.Base.UnitTests/Media/FontManagerTests.cs index ed95773630..d63f238dc1 100644 --- a/tests/Avalonia.Base.UnitTests/Media/FontManagerTests.cs +++ b/tests/Avalonia.Base.UnitTests/Media/FontManagerTests.cs @@ -1,4 +1,6 @@ using System; +using System.Threading; +using System.Threading.Tasks; using Avalonia.Media; using Avalonia.UnitTests; using Xunit; @@ -86,5 +88,54 @@ namespace Avalonia.Base.UnitTests.Media Assert.Equal("DejaVu", FontManager.Current.DefaultFontFamily.Name); } } + + [Fact] + public async Task TryGetGlyphTypeface_Should_Be_Thread_Safe_For_Embedded_Fonts() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var fontManager = FontManager.Current; + + const string fontUri = + "resm:Avalonia.Base.UnitTests.Assets?assembly=Avalonia.Base.UnitTests#Noto Mono"; + var collectionKey = + new Uri("resm:Avalonia.Base.UnitTests.Assets?assembly=Avalonia.Base.UnitTests"); + + // Warm up to validate the font URI is correct. + Assert.True(fontManager.TryGetGlyphTypeface(new Typeface(new FontFamily(fontUri)), out _)); + + const int iterations = 50; + int failures = 0; + + for (int i = 0; i < iterations; i++) + { + fontManager.RemoveFontCollection(collectionKey); + + using var barrier = new Barrier(2); + bool r1 = false, r2 = false; + + var t1 = Task.Run(() => + { + barrier.SignalAndWait(); + r1 = fontManager.TryGetGlyphTypeface(new Typeface(new FontFamily(fontUri)), out _); + }, TestContext.Current.CancellationToken); + + var t2 = Task.Run(() => + { + barrier.SignalAndWait(); + r2 = fontManager.TryGetGlyphTypeface(new Typeface(new FontFamily(fontUri)), out _); + }, TestContext.Current.CancellationToken); + + await Task.WhenAll(t1, t2); + + if (!r1 || !r2) + { + Interlocked.Increment(ref failures); + } + } + + Assert.Equal(0, failures); + } + } } } diff --git a/tests/Avalonia.Base.UnitTests/Media/FontManagerTryGetFontCollectionTests.cs b/tests/Avalonia.Base.UnitTests/Media/FontManagerTryGetFontCollectionTests.cs new file mode 100644 index 0000000000..2f85f0b63f --- /dev/null +++ b/tests/Avalonia.Base.UnitTests/Media/FontManagerTryGetFontCollectionTests.cs @@ -0,0 +1,263 @@ +using System; +using Avalonia.Media; +using Avalonia.Media.Fonts; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Base.UnitTests.Media +{ + public class FontManagerTryGetFontCollectionTests + { + [Fact] + public void TryGetFontCollection_SystemFontScheme_ReturnsTrue() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri($"{FontManager.SystemFontScheme}:Arial", UriKind.Absolute); + + Assert.True(FontManager.Current.TryGetFontCollection(source, out var collection)); + Assert.NotNull(collection); + } + } + + [Fact] + public void TryGetFontCollection_SystemFontScheme_YieldsSystemFontCollection() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri($"{FontManager.SystemFontScheme}:Arial", UriKind.Absolute); + + FontManager.Current.TryGetFontCollection(source, out var collection); + + Assert.IsType(collection); + } + } + + [Fact] + public void TryGetFontCollection_SystemFontScheme_ReturnsSameInstanceOnSubsequentCalls() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri($"{FontManager.SystemFontScheme}:Arial", UriKind.Absolute); + var fm = FontManager.Current; + + fm.TryGetFontCollection(source, out var first); + fm.TryGetFontCollection(source, out var second); + + Assert.Same(first, second); + } + } + + [Fact] + public void TryGetFontCollection_SystemFontsKey_ReturnsTrue() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + Assert.True(FontManager.Current.TryGetFontCollection(FontManager.SystemFontsKey, out var collection)); + Assert.NotNull(collection); + } + } + + [Fact] + public void TryGetFontCollection_SystemFontsKey_YieldsSystemFontCollection() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + FontManager.Current.TryGetFontCollection(FontManager.SystemFontsKey, out var collection); + + Assert.IsType(collection); + } + } + + [Fact] + public void TryGetFontCollection_SystemFontSchemeAndSystemFontsKey_ReturnSameInstance() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var fm = FontManager.Current; + var schemeSource = new Uri($"{FontManager.SystemFontScheme}:Arial", UriKind.Absolute); + + fm.TryGetFontCollection(schemeSource, out var fromScheme); + fm.TryGetFontCollection(FontManager.SystemFontsKey, out var fromKey); + + Assert.Same(fromScheme, fromKey); + } + } + + [Fact] + public void TryGetFontCollection_RegisteredFontsCollection_ReturnsTrue() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var key = new Uri("fonts:MyTest", UriKind.Absolute); + var stub = new StubFontCollection(key); + var fm = FontManager.Current; + fm.AddFontCollection(stub); + + Assert.True(fm.TryGetFontCollection(key, out var collection)); + Assert.Same(stub, collection); + } + } + + [Fact] + public void TryGetFontCollection_UnregisteredFontsCollection_ReturnsFalse() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var key = new Uri("fonts:DoesNotExist", UriKind.Absolute); + + Assert.False(FontManager.Current.TryGetFontCollection(key, out var collection)); + Assert.Null(collection); + } + } + + [Fact] + public void TryGetFontCollection_UnregisteredFontsCollection_DoesNotCacheNull() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var key = new Uri("fonts:DoesNotExist2", UriKind.Absolute); + var fm = FontManager.Current; + + // First call returns false + Assert.False(fm.TryGetFontCollection(key, out _)); + + // Register after the first failed lookup + var stub = new StubFontCollection(key); + fm.AddFontCollection(stub); + + // Now it should be found if null had been cached this would still fail + Assert.True(fm.TryGetFontCollection(key, out var collection)); + Assert.Same(stub, collection); + } + } + + [Fact] + public void TryGetFontCollection_AbsoluteResm_ReturnsTrue() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri("resm:Avalonia.Base.UnitTests.Assets?assembly=Avalonia.Base.UnitTests", UriKind.Absolute); + + Assert.True(FontManager.Current.TryGetFontCollection(source, out var collection)); + Assert.NotNull(collection); + } + } + + [Fact] + public void TryGetFontCollection_AbsoluteResm_YieldsEmbeddedFontCollection() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri("resm:Avalonia.Base.UnitTests.Assets?assembly=Avalonia.Base.UnitTests", UriKind.Absolute); + + FontManager.Current.TryGetFontCollection(source, out var collection); + + Assert.IsType(collection); + } + } + + [Fact] + public void TryGetFontCollection_AbsoluteResm_ReturnsSameInstanceOnSubsequentCalls() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri("resm:Avalonia.Base.UnitTests.Assets?assembly=Avalonia.Base.UnitTests", UriKind.Absolute); + var fm = FontManager.Current; + + fm.TryGetFontCollection(source, out var first); + fm.TryGetFontCollection(source, out var second); + + Assert.Same(first, second); + } + } + + [Fact] + public void TryGetFontCollection_Avares_ReturnsTrue() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri("avares://Avalonia.Base.UnitTests/Assets", UriKind.Absolute); + + Assert.True(FontManager.Current.TryGetFontCollection(source, out var collection)); + Assert.NotNull(collection); + } + } + + [Fact] + public void TryGetFontCollection_Avares_YieldsEmbeddedFontCollection() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri("avares://Avalonia.Base.UnitTests/Assets", UriKind.Absolute); + + FontManager.Current.TryGetFontCollection(source, out var collection); + + Assert.IsType(collection); + } + } + + [Fact] + public void TryGetFontCollection_Avares_ReturnsSameInstanceOnSubsequentCalls() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri("avares://Avalonia.Base.UnitTests/Assets", UriKind.Absolute); + var fm = FontManager.Current; + + fm.TryGetFontCollection(source, out var first); + fm.TryGetFontCollection(source, out var second); + + Assert.Same(first, second); + } + } + + [Fact] + public void TryGetFontCollection_UnknownScheme_ReturnsFalse() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + var source = new Uri("https://example.com/fonts", UriKind.Absolute); + + Assert.False(FontManager.Current.TryGetFontCollection(source, out var collection)); + Assert.Null(collection); + } + } + + [Fact] + public void TryGetFontCollection_UnknownScheme_DoesNotCacheNull() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface)) + { + // Verify that repeated lookups for the same unknown-scheme URI + // consistently return false/null rather than succeeding due to an + // accidentally cached null or invalid entry. + var source = new Uri("file:///some/path/fonts", UriKind.Absolute); + var fm = FontManager.Current; + + Assert.False(fm.TryGetFontCollection(source, out var first)); + Assert.Null(first); + + Assert.False(fm.TryGetFontCollection(source, out var second)); + Assert.Null(second); + } + } + + private sealed class StubFontCollection : IFontCollection + { + public StubFontCollection(Uri key) => Key = key; + + public Uri Key { get; } + public int Count => 0; + public FontFamily this[int index] => throw new NotSupportedException(); + public bool TryGetGlyphTypeface(string familyName, FontStyle style, FontWeight weight, FontStretch stretch, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out GlyphTypeface? glyphTypeface) { glyphTypeface = null; return false; } + public bool TryMatchCharacter(int codepoint, FontStyle style, FontWeight weight, FontStretch stretch, string? familyName, System.Globalization.CultureInfo? culture, out Typeface typeface) { typeface = default; return false; } + public bool TryGetFamilyTypefaces(string familyName, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out System.Collections.Generic.IReadOnlyList? familyTypefaces) { familyTypefaces = null; return false; } + public bool TryCreateSyntheticGlyphTypeface(GlyphTypeface glyphTypeface, FontStyle style, FontWeight weight, FontStretch stretch, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out GlyphTypeface? syntheticGlyphTypeface) { syntheticGlyphTypeface = null; return false; } + public bool TryGetNearestMatch(string familyName, FontStyle style, FontWeight weight, FontStretch stretch, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out GlyphTypeface? glyphTypeface) { glyphTypeface = null; return false; } + public System.Collections.Generic.IEnumerator GetEnumerator() => System.Linq.Enumerable.Empty().GetEnumerator(); + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + public void Dispose() { } + } + } +}