[Text] Make sure TryGetGlyphTypeface does not fail for concurrent access (#21269)

* Make sure TryGetGlyphTypeface does not fail for concurrent access

* Update tests/Avalonia.Base.UnitTests/Media/FontManagerTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update gitignore

* Make TryGetFontCollection more robust and add unit tests

* Introdcue GetOrCreateFontCollection helper for safe disposal of losing instances

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

.gitignore

@@ -221,3 +221,6 @@ src/Browser/Avalonia.Browser/wwwroot
 api/diff
 src/Browser/Avalonia.Browser/staticwebassets
 .serena
+
+# Claude agent worktrees
+.claude/worktrees/

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;
+        }
+
+        /// <summary>
+        /// Thread-safe get-or-create that disposes any candidate that loses the insertion race,
+        /// preventing resource leaks that <see cref="ConcurrentDictionary{TKey,TValue}.GetOrAdd(TKey,Func{TKey,TValue})"/>
+        /// can cause when the factory is invoked concurrently by multiple threads.
+        /// </summary>
+        private IFontCollection GetOrCreateFontCollection<TState>(Uri key, TState state, Func<Uri, TState, IFontCollection> 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)

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);
+            }
+        }
     }
 }

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<SystemFontCollection>(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<SystemFontCollection>(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<EmbeddedFontCollection>(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<EmbeddedFontCollection>(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<Typeface>? 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<FontFamily> GetEnumerator() => System.Linq.Enumerable.Empty<FontFamily>().GetEnumerator();
+            System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator();
+            public void Dispose() { }
+        }
+    }
+}