From 31a855b9b0b3aec18ec7ae835e3635b42ac98b2e Mon Sep 17 00:00:00 2001 From: Tobias Melcher Date: Tue, 16 Jun 2026 19:31:01 +0200 Subject: [PATCH] Fix IllegalArgumentException when drawing unified diff code minings UnifiedDiffLineHeaderCodeMining.draw() caches Font references in its foregrounds list and in cachedFont. These fonts can be disposed under the cache when: 1. The mining is disposed while a paint event for it is still in flight (e.g. "hide diff" recomputes the mining list and a pending paintControl still references the old annotations). 2. The widget font is replaced by an instance with equal FontData. The previous invalidation used Font.equals(), which is a value compare and does not detect a disposed handle. Once an exception was thrown, the stale cache was never refreshed, so every subsequent scroll repaint produced another error. Fix: - dispose() nulls the cached state so a stray paint after disposal cannot enter the fast path. - Cache invalidation also checks cachedFont.isDisposed(). - The fast-path loop validates f.font.isDisposed() before use; if stale, the cache is dropped and the slow path rebuilds it in the same paint. Fixes https://github.com/eclipse-platform/eclipse.platform/issues/2772 --- .../internal/AcceptAllRunnable.java | 6 ++++ .../internal/HideAllDiffsRunnable.java | 3 ++ .../unifieddiff/internal/KeepAllRunnable.java | 3 ++ .../unifieddiff/internal/NextRunnable.java | 3 ++ .../internal/PreviousRunnable.java | 3 ++ .../unifieddiff/internal/UndoAllRunnable.java | 6 ++++ .../UnifiedDiffCodeMiningProvider.java | 31 +++++++++++++------ .../internal/UnifiedDiffManager.java | 2 +- 8 files changed, 47 insertions(+), 10 deletions(-) diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/AcceptAllRunnable.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/AcceptAllRunnable.java index 0b54c658d43..9eb6b8c99fd 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/AcceptAllRunnable.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/AcceptAllRunnable.java @@ -61,6 +61,9 @@ public static ImageDescriptor getUndoImageDescriptor() { public void run() { StyledText tw = tv.getTextWidget(); List diffs1 = get(tv); + if (diffs1 == null) { + return; + } List positions = new ArrayList<>(); List replaceStrings = new ArrayList<>(); for (UnifiedDiff diff : diffs1) { @@ -84,6 +87,9 @@ public void run() { ext.updateCodeMinings(); } disposeUnifiedDiff(tv, model, tv.getTextWidget()); + if (positions.isEmpty()) { + return; + } // we have to insert with delay because otherwise the line header code minings // cannot be deleted runAfterRepaintFinished(tw, () -> { diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/HideAllDiffsRunnable.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/HideAllDiffsRunnable.java index 5b34dbf6516..daf1eb91ba1 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/HideAllDiffsRunnable.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/HideAllDiffsRunnable.java @@ -63,6 +63,9 @@ public void run() { removeAnnotationModelListener(model, tv.getTextWidget()); } List diffs1 = get(tv); + if (diffs1 == null) { + return; + } for (UnifiedDiff diff : diffs1) { List annos = getAllAnnotationsForUnifiedDiff(model, diff); for (var lanno : annos) { diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/KeepAllRunnable.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/KeepAllRunnable.java index fe39a4cda04..4884f363691 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/KeepAllRunnable.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/KeepAllRunnable.java @@ -42,6 +42,9 @@ public String getLabel() { @Override public void run() { List diffs1 = get(tv); + if (diffs1 == null) { + return; + } for (UnifiedDiff diff : diffs1) { List annos = getAllAnnotationsForUnifiedDiff(model, diff); for (var lanno : annos) { diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/NextRunnable.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/NextRunnable.java index f6fd00f6de6..687480d33b2 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/NextRunnable.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/NextRunnable.java @@ -62,6 +62,9 @@ public void run() { } int offset = sel.getOffset(); List diffs1 = get(tv); + if (diffs1 == null || diffs1.size() == 0) { + return; + } // get next UnifiedDiff for given offset UnifiedDiff nextDiff = null; for (UnifiedDiff diff : diffs1) { diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/PreviousRunnable.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/PreviousRunnable.java index a4647dc0c0b..71375648c44 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/PreviousRunnable.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/PreviousRunnable.java @@ -62,6 +62,9 @@ public void run() { } int offset = sel.getOffset(); List diffs1 = get(tv); + if (diffs1 == null || diffs1.size() == 0) { + return; + } // get next UnifiedDiff for given offset UnifiedDiff nextDiff = null; for (UnifiedDiff diff : diffs1) { diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UndoAllRunnable.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UndoAllRunnable.java index dbaaf890687..2b82eb8a893 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UndoAllRunnable.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UndoAllRunnable.java @@ -49,6 +49,9 @@ public String getLabel() { public void run() { var tw = tv.getTextWidget(); List diffs1 = get(tv); + if (diffs1 == null) { + return; + } List positions = new ArrayList<>(); List replaceStrings = new ArrayList<>(); for (UnifiedDiff diff : diffs1) { @@ -72,6 +75,9 @@ public void run() { ext.updateCodeMinings(); } disposeUnifiedDiff(tv, model, tv.getTextWidget()); + if (positions.isEmpty()) { + return; + } // we have to insert with delay because otherwise the line header code minings // cannot be deleted runAfterRepaintFinished(tw, () -> { diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java index 73897c49032..7fb75205292 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java @@ -613,7 +613,15 @@ public UnifiedDiff getUnifiedDiff() { @Override public void dispose() { + cleanCachedData(); super.dispose(); + } + + private void cleanCachedData() { + foregrounds = null; + backgrounds = null; + lastRectangle = null; + cachedFont = null; clearStyledFonts(); } @@ -634,14 +642,13 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { gc.setForeground(c); Font font = textWidget.getFont(); gc.setFont(font); - if (cachedFont != null && !cachedFont.equals(font)) { + if (cachedFont != null && (cachedFont.isDisposed() || !cachedFont.equals(font))) { // font might have been changed in the meantime - remove cache - lastRectangle = null; - backgrounds = null; - foregrounds = null; - clearStyledFonts(); + cleanCachedData(); } + cachedFont = font; if (lastRectangle != null && backgrounds != null && foregrounds != null) { + boolean fontIsDisposed = false; // draw background // vs code is drawing the background to the top right of the editor - we do here // the same! @@ -663,20 +670,26 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { } if (f.font == null) { gc.setFont(cachedFont); - } else { + } else if (!f.font.isDisposed()) { gc.setFont(f.font); + } else { + cleanCachedData(); + cachedFont = font; + fontIsDisposed = true; + break; } gc.setBackground(f.background); gc.setForeground(f.foreground); gc.drawString(f.str, x + f.x, y + f.y, true); } - lastRectangle = new Rectangle(x, y, lastRectangle.width, lastRectangle.height); - return new Point(lastRectangle.width, lastRectangle.height); + if (!fontIsDisposed) { + lastRectangle = new Rectangle(x, y, lastRectangle.width, lastRectangle.height); + return new Point(lastRectangle.width, lastRectangle.height); + } } // first run to get width and height for label Point result = super.draw(gc, textWidget, color, x, y); lastRectangle = new Rectangle(x, y, result.x, result.y); - cachedFont = font; // draw background // vs code is drawing the background to the top right of the editor - we do here diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java index 179edb1a2d9..52995417231 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java @@ -470,8 +470,8 @@ private static void drawToolBarForAllDiffs(ITextViewer tv, IAnnotationModel mode UnifiedDiffMode mode) { StyledText tw = tv.getTextWidget(); var tm = new ToolBarManager(SWT.FLAT | SWT.HORIZONTAL | SWT.RIGHT); - List diffs = get(tv); if (UnifiedDiffMode.OVERLAY_MODE.equals(mode) || UnifiedDiffMode.OVERLAY_READ_ONLY_MODE.equals(mode)) { + List diffs = get(tv); if (!isReadOnly(diffs)) { var acceptAll = new AcceptAllRunnable(tv, model); addToolbarAction(tm, acceptAll.getLabel(), AcceptAllRunnable.getImageDescriptor(), acceptAll);