Skip to content

Conversation

@arunjose696
Copy link
Contributor

@arunjose696 arunjose696 commented Jul 22, 2025

This change ensures font is fetched from registry even when zoom levels match.

The font returned by Font.win32_new is stored in the setFont() method of widgets. Previously, if targetZoom == font.zoom, the passed font was returned directly. However, the passed font could have been disposed by user code. This change ensures that the font is always fetched from the font registry, even when the zoom levels match, avoiding potential use of disposed fonts.

Steps to reproduce

The below snippet fails on master. And passes with the change

package org.eclipse.swt.snippets;
import org.eclipse.swt.*;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.widgets.*;

public class CanvasCaretFocus {
    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setSize(300, 200);
        shell.setText("Canvas Caret Focus");
        Canvas canvas = new Canvas(shell, SWT.BORDER);

        shell.open();
        Caret caret = new Caret(canvas, SWT.NONE);
        Font font = new Font(display, "Default", 10, SWT.BOLD);
    	caret.setFont(font );
    	font.dispose();

    	canvas.setFocus();
        canvas.setCaret(caret);



    }
}

@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2025

Test Results

   539 files   -  7     539 suites   - 7   30m 23s ⏱️ +57s
 4 358 tests  - 54   4 318 ✅  - 77   14 💤  - 3  25 ❌ +25  1 🔥 +1 
16 664 runs   - 54  16 514 ✅  - 77  124 💤  - 3  25 ❌ +25  1 🔥 +1 

For more details on these failures and errors, see this check.

Results for commit 7f78dde. ± Comparison against base commit 7b46268.

This pull request removes 54 tests.
AllWin32Tests ImageWin32Tests ‑ testDisposeDrawnImageBeforeRequestingTargetForOtherZoom
AllWin32Tests ImageWin32Tests ‑ testDrawImageAtDifferentZooms(boolean)[1] true
AllWin32Tests ImageWin32Tests ‑ testDrawImageAtDifferentZooms(boolean)[2] false
AllWin32Tests ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
AllWin32Tests ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
AllWin32Tests ImageWin32Tests ‑ testRetrieveImageDataAtDifferentZooms(boolean)[1] true
AllWin32Tests ImageWin32Tests ‑ testRetrieveImageDataAtDifferentZooms(boolean)[2] false
AllWin32Tests TestTreeColumn ‑ test_ColumnOrder
AllWin32Tests Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
AllWin32Tests Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
…

♻️ This comment has been updated with latest results.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you already have a simple reproducing snippet, maybe you could use that to add a regression test, so that we might see such a bug more early in the future?

@arunjose696 arunjose696 marked this pull request as draft July 23, 2025 09:06
@arunjose696 arunjose696 force-pushed the arunjose696/368/FontDisposal branch 2 times, most recently from 599383d to ecb1756 Compare July 23, 2025 14:18
The font returned by Font.win32_new is stored in the setFont() method of
widgets. Previously, if targetZoom == font.zoom, the passed font was
returned directly. However, the passed font could have been disposed by
user code. This change ensures that the font is always fetched from the
font registry, even when the zoom levels match, avoiding potential use
of disposed fonts.
@arunjose696 arunjose696 force-pushed the arunjose696/368/FontDisposal branch from ecb1756 to 7f78dde Compare July 25, 2025 08:42
@arunjose696
Copy link
Contributor Author

Closing this as I have opened a different PR which fixes this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SWTException: Font with disposed graphics

2 participants