Fix duplicate-key crash on BBCode IsBold font resolution (#3530)#3531
Merged
Conversation
Assigning a TextRuntime.Text containing a BBCode [IsBold=true] run could crash with "An item with the same key has already been added" for the bold font's FontCache .fnt key. The inline-BBCode font resolver (CustomSetPropertyOnRenderable. GetAndCreateFontIfNecessary) resolves the same font once per open/close tag by design and dedups only via GetDisposable(...) as BitmapFont. When an earlier resolution cached a value that cast cannot recover (a null or shared Text.DefaultBitmapFont fallback cached because no .fnt was on disk yet), the guard sees an empty slot, falls through, and AddDisposable collides with the still-occupied key. AddDisposable defaulted to ExistingContentBehavior.ThrowException, so it threw. Pass ExistingContentBehavior.Replace at both AddDisposable calls in GetAndCreateFontIfNecessary so a re-resolution heals the poisoned slot instead of crashing, matching the protection already present on the raylib and non-BBCode MonoGame font paths. Replace is used rather than Dispose-then-add because the occupying value can be null or the shared default font, which LoaderManager.Dispose cannot safely handle. Adds MonoGameGum.Tests regression coverage.
vchelaru
approved these changes
Jul 7, 2026
Supplements the regression test: adds a second test exercising the in-memory font-creation AddDisposable site (the disk/DefaultBitmapFont fallback site was already covered), plus a heal assertion proving the poisoned slot is replaced so the test can't pass vacuously if the computed cache key drifts. Verified red-for-the-right-reason per line; full MonoGameGum.Tests suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3530.
Problem
Assigning a
TextRuntime.Textcontaining a BBCode[IsBold=true]run could crash withSystem.ArgumentException: An item with the same key has already been addedfor the bold font's FontCache.fntkey. Hit by a shipping FlatRedBall game on a wave-complete message — the "crashes on 2nd load" symptom.Root cause
The inline-BBCode font resolver (
CustomSetPropertyOnRenderable.GetAndCreateFontIfNecessary) resolves the same font once per open/close tag and dedups viaLoaderManager.GetDisposable(...) as BitmapFont. That dedup is unsound:GetDisposablereturnsnullboth when the key is absent and when the key is present but holdsnull— it cannot tell them apart.The BBCode path caches whatever it resolves under the font key, including its last-resort
Text.DefaultBitmapFontfallback. When that fallback is itselfnull— as under FRB, where Gum's default bitmap font is never assigned — the first resolution caches anullunder the bold key. The next resolution seesGetDisposable(...) as BitmapFont == null, treats the occupied slot as empty, falls through, and callsAddDisposable, which defaulted toExistingContentBehavior.ThrowExceptionand threw on the already-present key. (A non-null cached font — including a non-nullDefaultBitmapFont— is recovered by the cast and never triggers this; only a cachednullpoisons the slot.)The non-BBCode MonoGame path sidesteps this by never caching the fallback — it applies
?? Text.DefaultBitmapFonttransiently without caching. The inline-BBCode path was the one spot that cached the fallback and then re-added under the same key unguarded.Fix
Pass
ExistingContentBehavior.Replaceat bothAddDisposablecalls inGetAndCreateFontIfNecessary, so a re-resolution heals the occupied slot instead of crashing.Replace(rather thanDispose-then-add) is required because the occupying value can benullor the shared default font, neither of whichLoaderManager.Disposecan safely handle.Tests
MonoGameGum.Tests/Runtimes/TextRuntimeBbCodeFontCachingRegressionTests.cspoisons the exact bold cache key so the dedup guard sees an empty slot, then asserts a[IsBold=true]Text assignment no longer throws and that the slot is actually replaced with aBitmapFont(so the test can't pass vacuously if the computed key ever drifts). Two tests cover the twoAddDisposablesites the fix touches: the disk/DefaultBitmapFontfallback (the crash's exact line) and in-memory font creation. Each was confirmed to fail for the right reason (the duplicate-keyArgumentException) by reverting its fix line independently; the fullMonoGameGum.Testssuite is green.