Resource leak in VControl
gbburkhardt opened this issue · 10 comments
The dispose() method of VControl should dispose of the Image if 'image' is not null.
@gbburkhardt can you explain more, e.g. give an example snippet or reference the code? In general if you set an image you should dispose it (e.g. with a dispose listener) and not the control.
I discovered this when using the CDateTime widget. One could argue that CDateTime should dispose of the image passed to 'timeButton' at line 586 of DatePicker. There is code to dispose of the image provided by org.eclipse.nebula.widgets.cdatetime.Resources, but that will only be called when an application exits. If a CDateTime widget is created, and then disposed of, the image GDI object lingers. The 'dispose' method of VControl is called when the widget is destroyed, so it's safer to put the image dispose there. Calling dispose on an Image that has been disposed does nothing.
As I understand, a control should only dispose of an image, or any other
resource, it it has created the resource.
Exactly!
Ok, so you choose that CDateTime has the resource leak. That's fine with me.
The root of the problem is in org.eclipse.nebula.widgets.cdatetime.Resources. Its disposeListener is only triggered when the display is disposed, which will typically only happen when the program exits, so what's the point? Any widget in the CDateTime collection that uses Resources to create an image needs to have a disposeListner created on the widget (e.g., CDateTime, DatePicker), that calls a dispose method in Resources. It looks to me that everywhere Resources.getImage() is called is leaking GDI objects, when the widget is destroyed.
@dplaliberte This is the philosophy of SWT : if you set an image to a Button (for example), when you dispose the button the associated image is not disposed.
@gbburkhardt I'll have a look at this bug
@gbburkhardt It is not clear for me (baybe @lcaron knows better) who creates the button, also you are talking about VControl in the title but say there is a leak in CDateTime, so it is unclear for me who is creating the image and why do you think there is a leak, usually images are keept around if they are shared, maybe up to the life-time of the application to not create them for each control individually.
There are several methods in Resources that create images, and then cache the created Image for reuse. See all the Resources.getIconXXXX() methods. I don't see any of those images being disposed when either CDateTime or DatePicker is disposed. There's a minor complication in that all those methods are static as is the cache, so if both CDateTime and DatePicker were to use the same image, one wouldn't want to dispose of the image when DatePicker is disposed.
Probably Resources could use an disposeImage(String) to get rid of a particular image if it was cached.
If the images are shared they are also used to reuse them, disposing them would defeat the whole purpose of this.
Except you assume that an application can only ever have exactly one CDateTime and never create one again.
@laeubi : that's a good point. So much for shared images.
BTW, the Resources.getIconXXXX() methods will always create an image if none exists in the cache. But if multiple instances of CDateTime are in use, disposing of any of the images is an issue.
So I withdraw my original complaint, since only a limited number of images will be created during the lifetime of an application, and new instances of CDateTime won't create new ones.
Thanks.