From 81ae8d97a4fb24a858f8adc24b64faba6bb1687e Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Wed, 19 Aug 2026 16:03:35 +0200 Subject: [PATCH] Make FontRegistry's table of font data thread safe FontRegistry keeps its symbolic-name-to-FontData mapping in a plain HashMap that is read and written from arbitrary threads: - put(String, FontData[]) is public and carries no UI-thread restriction, unlike the methods that hand out Font instances, - getFontData(), getDescriptor(), getKeySet() and hasValueFor() read it and are likewise unrestricted, with getKeySet() even handing out a live view of the table, and - createFont() writes to it (via the internal put() overload) from whichever thread realizes a font, i.e. from any SWT Display's thread. Unsynchronized HashMap mutation from several threads can corrupt the table itself, not merely produce stale reads. Use a ConcurrentHashMap instead, so concurrent access is safe and getKeySet() returns a weakly-consistent view rather than one that may fail arbitrarily while being iterated. Also fold the internal put()'s read-compare-write of that table into a single atomic Map#put: the previous get()/put() pair could interleave so that two threads both concluded the mapping was unchanged, or that the mapping they replaced was one that had already been overwritten. Storing the given array when it is content-equal to the existing one is a no-op for every reader, so this does not change behavior. The table of realized FontRecords is deliberately left alone here: it is guarded by the documented UI-thread restriction of the methods returning Font instances. Assisted-by: Claude Opus 5 --- .../src/org/eclipse/jface/resource/FontRegistry.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java index ec360f9dcca..baf1e92f66d 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java @@ -196,7 +196,7 @@ void addAllocatedFontsToStale(Font defaultFont) { * (key type: String, * value type: org.eclipse.swt.graphics.FontData[]). */ - private final Map stringToFontData = new HashMap<>(7); + private final Map stringToFontData = new ConcurrentHashMap<>(7); /** * Collection of Fonts that are now stale to be disposed @@ -816,14 +816,15 @@ private void put(String symbolicName, FontData[] fontData, boolean update) { Assert.isNotNull(symbolicName); Assert.isNotNull(fontData); - FontData[] existing = stringToFontData.get(symbolicName); + // single atomic read-modify-write; replacing an equal mapping with the + // given, content-equal one is a no-op for every reader + FontData[] existing = stringToFontData.put(symbolicName, fontData); if (Arrays.equals(existing, fontData)) { return; } FontRecord oldFont = stringToFontRecord .remove(symbolicName); - stringToFontData.put(symbolicName, fontData); if (update) { fireMappingChanged(symbolicName, existing, fontData); }