OutOfMemory when disposing images due to many copies of image in GC.operations

[meyertom]

Describe the bug
We are using dynamic images that are then displayed on a Canvas for a process-state visualization.
The addition of the operations list in GC for ImageOperations is a problem in our case. the list contains all (Image)Operations since the opening of the Window, leading to a very long list of operations acting on the same image(s). When such an image is disposed, a copy of the image is made for every ImageOperation (acting on this image) contained in the operations list of the gc.
In our setup this leads to thousands of copies of a single image, that are created on disposal of the image.

Inverting the disposal order is not an option in our case,

I have no clear understanding on when the operations list is used in GC (it seems to be used only when GC. refreshFor(Drawable drawable, ImageHandle imageHandle is invoked. Therefore I'm not in a position at the moment to suppose a solution for the problem.
Removing old image operations from operations , when the image that is drawn and the boundaries are already contained in operatons might be an idea but the logic when an operation in operations might be removed might get complicated.

To Reproduce
Create an image.
Draw the image to a Canvas
modify the image
redraw the image in the Canvas.
dispose the image
This leads to two copies of the image beeing created in the ImageOperations of the GC.

Expected behavior
Only operations that help determine the state of the Drawable should remain in GC.operations.

Screenshots
If applicable, add screenshots to help explain your problem.

Environment:

1. Select the platform(s) on which the behavior is seen:

• • All OS
• • Windows
• • Linux
• • macOS

2. Additional OS info (e.g. OS version, Linux Desktop, etc)

3. JRE/JDK version

Version since
We encontered this problem whe switching to SWT 4.39, but te probelm exists since the introduction of the operations list in GC.

Workaround (or) Additional context
Add any other context about the problem here.
Any known workarounds for the problem?

[iloveeclipse]

If I understand it right , you refer to this change: #2331 and it is Windows specific issue.

@HeikoKlare , could you please check?

[iloveeclipse]

@meyertom : a self contained example which shows you problem would be for sure helpful.

[HeikoKlare]

Thank you for reporting that issue. A reproducer for an actual OutOfMemory error would indeed be useful. In case you can share a reproducer, I will have a look into it as soon as possible. In case it turns out that we have an actual memory leak here, the issue would be quite severe. Note that we are already quite close to the next releae (this week is M3 for the Platform), so it would be great if we could analyze the issue in the next days.

The described reproducer is valid insofar as drawing an image twice with a GC and disposing it aftwards will lead to two copies of the image in the GC operations. This is as intended: a copy of every disposed image drawn to a GC needs to be preserved such that the GC can be refreshed (upon zoom change of the target context). Such images may only be disposed once the GC itself is disposed as otherwise a proper refresh of the GC becomes impossible.
In the described case it does not make that much sense that the GC copies both images because the copies created upon image disposal will only represent the final state of the image after all modifications to it (so one copy would be sufficient). Note that this shows why it is quite dangerous to modify an image that was already drawn to a GC, because a refresh of the GC (upon zoom change of the target context) potentially leads to the modified version being printed at the place where the original, unmodified version of the image is supposed to be printed. The cleaner (or rather the correct) approach would be to create a new image or modify a copy of the original one respectively. In any case, the reported issues seems to be about so many image copies that it should not matter if one or two copies of the same image are preserved.

That said, I have to admit that I currently don't understand why this temporary image copy should produce an OutOfMemory error. Do you manually create a GC for that Canvas and unnecessarily keep that alive for a long time such that the copied images sum up endlessly? If that was the case, the way you draw on a Canvas would not be as intended. Drawing on a Control should be done via a paint listener and not by manually creating a GC on the Control, and such a GC (including the temporary images contained in its operations) is automatically disposed once redraw of the Control is finished. If you are already doing that, the question is why thousands of images are generated during a single redraw operation of a Canvas.

[meyertom]

Thank you for your support. You are right, we do indeed have long lived manually created GCs in our code. Those accumulate a substantial number of operations during their lifetime. Many of those operations are image operations, as a small number of images are modified by other sources and redrawn at their original positions, thus replacing the old state. Those GCs that are responsible for the copied images when one or more of the images is disposed (leading to a massive increase of the used memory). This code has been unmodified for years and has never been a problem until now. But I see that it is rathed a design flaw on our side than a real problem in swt.. The problem should never arise when using short lived GCs and copies of the relevant images.
What i still don't really see is when exactly the refresh of a GC is triggered. The operations list is applied only in those cases and until now I was never able to trigger this behavior. Even when changing the display scaling with the application running and displaying our canvas.
I will try to refactor the code on our side to use only the provided GCs of the paint listener or at least disposing them after use when creating them manually.We should be able to work around this issue without changes in swt, so you may consider this a non-issue.

[HeikoKlare]

What i still don't really see is when exactly the refresh of a GC is triggered. The operations list is applied only in those cases and until now I was never able to trigger this behavior. Even when changing the display scaling with the application running and displaying our canvas.

A refresh of a GC is always necessary when the rendering target is requested at a different zoom than for which the GC operations were originally applied. The most relevant example is the creation of a buffer image, for which you do not know the target context it will be drawn to upon its creation and upon application of GC operations. So you create a buffer image, create a GC on it, and perform GC operations. Then you draw the image onto some other drawable (via a GC on that drawable), which is usually a control, and only then you know the zoom at which the image is required (depending on the zoom of the target drawable).

That said, I have been further thinking about the issue yesterday and also noticed that storing GC operations is (currently) not necessary for every GC. It will only be relevant when you draw onto some drawable which may later be requested for a zoom that you don't know upon execution of the GC operations. This will probably never be the case for Controls (you know their current zoom) but only for Resources of which the only one that is a Drawable is the image. So I think we could probably (further) restrict the cases in which a GC stores operations (and in particular copies images).
Even though the change will probably be quite simple, I do not propose to do it for the upcoming release, as we are quite late in the deveopment cycle and I would like to have downstream projects that require the GC operations (like WindowBuilder) properly test such a change throughout a complete development cycle.

Even if that might fix your current issue, it's probably a good idea to refactor your code to have shorter-living GCs anyway. You will then still benefit from the improvement I will try to implement with the 2026-09 release when you update to it (or to an earlier I/M build of the release). Is that approach fine for your?

So thank you again for bringing this up. I think it revealed a great opportunity for improvement. Let's keep this issue open and mark it as resolved once we have an according improvement implemented.