Add missing image to shared images and resolve resource leak #3404

HeikoKlare · 2025-10-16T11:52:19Z

Currently, a shared image descriptor for a missing image is provided via the ImageDescriptor class. But images created for that descriptor still have to be maintained manually, e.g., by a resource manager or manual disposal. In some cases, this is not done correctly, leading to non-disposed resources.

This change adds a shared instance of the missing image that may be reused wherever needed. It is adopted for placeholders, which by now created missing images but never cleaned them up. That resource leak is resolved with the this change.

The proposal is driven by the following finding in an E3-based product:

java.lang.Error: SWT Resource was not properly disposed
	at org.eclipse.swt.graphics.Resource.initNonDisposeTracking(Resource.java:191)
	at org.eclipse.swt.graphics.Resource.<init>(Resource.java:124)
	at org.eclipse.swt.graphics.Image.<init>(Image.java:635)
	at org.eclipse.jface.resource.ImageDescriptor.createImage(ImageDescriptor.java:382)
	at org.eclipse.jface.resource.ImageDescriptor.createImage(ImageDescriptor.java:331)
	at org.eclipse.jface.resource.ImageDescriptor.createImage(ImageDescriptor.java:309)
	at org.eclipse.ui.internal.WorkbenchPage.handleNullRefPlaceHolders(WorkbenchPage.java:4049)
	at org.eclipse.ui.internal.WorkbenchPage.busySetPerspective(WorkbenchPage.java:3999)
	at org.eclipse.ui.internal.WorkbenchPage.lambda$12(WorkbenchPage.java:3969)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67)
	at org.eclipse.ui.internal.WorkbenchPage.setPerspective(WorkbenchPage.java:3969)
	at org.eclipse.ui.internal.Workbench.activate(Workbench.java:2990)
	at org.eclipse.ui.internal.Workbench.busyShowPerspective(Workbench.java:3031)
	at org.eclipse.ui.internal.Workbench.lambda$19(Workbench.java:3006)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67)
	at org.eclipse.ui.internal.Workbench.showPerspective(Workbench.java:3004)
	at org.eclipse.ui.internal.Workbench.showPerspective(Workbench.java:2979)

github-actions · 2025-10-16T12:37:41Z

Test Results

3 018 files ±0 3 018 suites ±0 2h 30m 54s ⏱️ - 3m 1s
8 226 tests ±0 7 975 ✅ +1 249 💤 ±0 2 ❌ - 1
23 598 runs ±0 22 802 ✅ +1 794 💤 ±0 2 ❌ - 1

For more details on these failures, see this check.

Results for commit 3adaaf8. ± Comparison against base commit 301653e.

♻️ This comment has been updated with latest results.

fedejeanne

The changes look good, thank you!

Just for my understanding: may this type of fix also be used in other locations? I ask because when I look at our Grafana board (I can provide you the link privately), I see other instances of Not properly disposed SWT resource, e.g. this one (happened in 4.37.0.v20250905-0730)

[ERROR] Not properly disposed SWT resource
java.lang.Error: SWT Resource was not properly disposed
	at org.eclipse.swt.graphics.Resource.initNonDisposeTracking(Resource.java:191)
	at org.eclipse.swt.graphics.Resource.<init>(Resource.java:117)
	at org.eclipse.swt.graphics.GC.<init>(GC.java:175)
	at org.eclipse.swt.graphics.GC.<init>(GC.java:143)
	at org.eclipse.jface.text.source.LineNumberRulerColumn.redraw(LineNumberRulerColumn.java:986)
	at org.eclipse.jface.text.source.LineNumberRulerColumn.lambda$0(LineNumberRulerColumn.java:407)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4135)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3751)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:678)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:583)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:185)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:219)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:149)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:115)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:467)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:298)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:627)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:575)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1431)

HeikoKlare · 2025-10-17T08:25:33Z

Just for my understanding: may this type of fix also be used in other locations?

The type of fix may be reasonable when you have images for which you do not have a clear responsibility for who needs to clean them up (and when) and they can reasonably be made shared images (thus permanently occupying a handle).
The given stack, however, shows a resource leak due to a GC. For that one I would expect that the doubleBufferPaint() operation line after creating the GC fails for some reason, as the GC is disposed in the subsequent line which, in my understanding, may just not happen if the previous line throws an exception:

eclipse.platform.ui/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/source/LineNumberRulerColumn.java

Lines 981 to 983 in 5b3382d

    
           GC gc= new GC(fCanvas); 
        
           doubleBufferPaint(gc); 
        
           gc.dispose();

fedejeanne · 2025-10-17T09:28:59Z

Ah, good point: I posted a resource leak due to a GC.

But the explanation still applies: this is a follow-up error for something that broke in-between initialization and disposal of the resource so changing the code in there makes little sense. One should simply fix the error that let doubleBufferPaint(gc) crash and the leakage would not happen.

Thank you for the clarification.

HeikoKlare · 2025-10-17T09:32:07Z

One should simply fix the error that let doubleBufferPaint(gc) crash and the leakage would not happen.

Yes, that's true. One should definitely fix the underlying issue. Still it's usually good practice to put code executed on a local resource (i.e., between resource initialization and disposal) into a try-finally block to ensure that in any case (in particular those exceptional cases not considered yet) resources are still cleaned up properly.

Currently, a shared image descriptor for a missing image is provided via the ImageDescriptor class. But images created for that descriptor still have to be maintained manually, e.g., by a resource manager or manual disposal. In some cases, this is not done correctly, leading to non-disposed resources. This change adds a shared instance of the missing image that may be reused wherever needed. It is adopted for placeholders, which by now created missing images but never cleaned them up. That resource leak is resolved with the this change.

HeikoKlare · 2025-10-17T14:43:57Z

Failing tests are unrelated and documented:

TestColumnWrapLayout#testHorizontalMargins and #testVerticalMargins fail on Windows #3413

HeikoKlare marked this pull request as ready for review October 16, 2025 13:22

HeikoKlare mentioned this pull request Oct 16, 2025

Resource leak due to missing image vi-eclipse/Eclipse-Platform#480

Closed

HeikoKlare linked an issue Oct 16, 2025 that may be closed by this pull request

Resource leak due to missing image vi-eclipse/Eclipse-Platform#480

Closed

fedejeanne approved these changes Oct 17, 2025

View reviewed changes

HeikoKlare added 2 commits October 17, 2025 14:28

Bump minor version of org.eclipse.ui.workbench for 4.38 stream

3adaaf8

HeikoKlare force-pushed the missingimage-resourceleak branch from 5a292f8 to 3adaaf8 Compare October 17, 2025 12:28

fedejeanne approved these changes Oct 17, 2025

View reviewed changes

HeikoKlare merged commit 8ff6a17 into eclipse-platform:master Oct 17, 2025
20 of 23 checks passed

HeikoKlare deleted the missingimage-resourceleak branch October 17, 2025 14:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add missing image to shared images and resolve resource leak #3404

Add missing image to shared images and resolve resource leak #3404

Uh oh!

HeikoKlare commented Oct 16, 2025

Uh oh!

github-actions bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

fedejeanne left a comment

Uh oh!

HeikoKlare commented Oct 17, 2025

Uh oh!

fedejeanne commented Oct 17, 2025

Uh oh!

HeikoKlare commented Oct 17, 2025

Uh oh!

HeikoKlare commented Oct 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add missing image to shared images and resolve resource leak #3404

Add missing image to shared images and resolve resource leak #3404

Uh oh!

Conversation

HeikoKlare commented Oct 16, 2025

Uh oh!

github-actions bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

HeikoKlare commented Oct 17, 2025

Uh oh!

fedejeanne commented Oct 17, 2025

Uh oh!

HeikoKlare commented Oct 17, 2025

Uh oh!

HeikoKlare commented Oct 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Oct 16, 2025 •

edited

Loading