Implement java.lang.AutoCloseable in class org.eclipse.swt.graphics.GC

[tobiasmelcher]

I suggest to implement the java.lang.AutoCloseable interface in org.eclipse.swt.graphics.GC so that the try-with-resources Java feature can be used.

Here a concrete example in org.eclipse.jface.dialogs.Dialog where the GC class is used.

protected void initializeDialogUnits(Control control) {
	// Compute and store a font metric
	GC gc = new GC(control);
	gc.setFont(JFaceResources.getDialogFont());
	fontMetrics = gc.getFontMetrics();
	gc.dispose();
}

gc.dispose() is not called when JFaceResources.getDialogFont() throws an exception.

Wouldn't it be much nicer if the try-with-resource approach would be used here:

protected void initializeDialogUnits(Control control) {
	// Compute and store a font metric
	try (GC gc = new GC(control)) {
	  gc.setFont(JFaceResources.getDialogFont());
	  fontMetrics = gc.getFontMetrics();
	}
}

Did you already discuss this topic? Are there any drawbacks of implementing the AutoCloseable interface in GC?

[vogella]

+1

[BeckerWdf]

Does anybody see unwanted side-effects when we would implement this proposal?

[merks]

I tried it locally a few minutes ago and checked if existing uses would get a warning about missing the close. New warnings that suggest close must be called would be very annoying...

[BeckerWdf]

I tried it locally a few minutes ago and checked if existing uses would get a warning about missing the close. New warnings that suggest close must be called would be very annoying...

And did new warnings happen?

[merks]

No, but I wasn't sure if that's because of some preferences. E.g.,

I'll test some more...

[merks]

Here is a self-contained example:

So yes, an auto closeable that isn't closed is considered a resource leak and depending on how a project configures the compiler preferences, this can be an error.

I don't think that's acceptable.

[tobiasmelcher]

Understood. We would introduce warnings or errors for existing code which does call dispose, but not using try-with-resources. We want to find all the locations which do not call dispose and introduce a SWT handle leak. Or make it easier for consumers so that dispose() must no longer be called explicitly.

[merks]

I fully understand the common/general use case of a GC needing to be disposed within the block of code in which it is created and see significant value in the suggestion from that point of view. It's just that when we talk about what "we" want, we ought to take into consideration everyone. Definitely not everyone will want new errors, warnings, or infos. Also, a great many of the "we" out there will want to provide libraries that work with older versions of SWT as well as with the latest version of SWT. This makes retrofitting good new patterns into the existing code a significant tradeoff for the huge downstream code base out there in the ecosystem.

[Phillipus]

Agree that it's a great idea but, like @merks, I implemented AutoCloseable in GC locally as a test and my workspace had several warnings about resource leaks. I could fix those in my own code but not in third-party code, so I would have to re-configure my workspace to turn the warnings/errors off which is not ideal.

[laeubi]

There are several ways to accomplish something similar for example one can have a static method in GC class like this:

public static void localGC(Drawable drawable, SWTRunnable runnable) {
	GC gc = new GC(drawable);
	try {
		runnable.run();
	} finally {
		gc.dispose();
	}
}

[HannesWell]

In general I like to leverage java language to increase the safety of the program and

I could fix those in my own code but not in third-party code, so I would have to re-configure my workspace to turn the warnings/errors off which is not ideal.

Do you have thrid-party code as sources in your workspace? Because for compiled jars such warnings are not shown. Maybe it would then be good to contribute to those projects (if they are open source).

There are several ways to accomplish something similar for example one can have a static method in GC class like this:

public static void localGC(Drawable drawable, SWTRunnable runnable) {
	GC gc = new GC(drawable);
	try {
		runnable.run();
	} finally {
		gc.dispose();
	}
}

I would name it GC.drawOn(drawable, ...) and I think this should be a consumer that accepts a gc, but in general this looks like a good suggestion. But then we should discourage from using the current constructor.

Definitely not everyone will want new errors, warnings, or infos.

I think nobody wants that, but applying a new method/class/etc. to the Eclipse SDK code base is often a good measure to see if it is really as good as hoped and to find potential flaws early. So ideally the designer also takes up that burden. :)

Also, a great many of the "we" out there will want to provide libraries that work with older versions of SWT as well as with the latest version of SWT. This makes retrofitting good new patterns into the existing code a significant tradeoff for the huge downstream code base out there in the ecosystem.

This is indeed a real difficulty and I understand that it is annoying for those that can't use the new things. But how do we then tell those that can use the new methods that there is something better to use? I doubt that that many scan the docs/classes for differences and only a fraction reads the N&N especially if there are tutorials out in the internet that are decades old. But a (deprecation, resource-leak, what ever) warning is often a good way to make people aware of a better alternative.

[jukzi]

you would need to

• org.eclipse.swt.graphics.Resource implements AutoCloseable,
• deprecated dispose() for removal
• refactor all usages of dispose() with .close() - easy
• wait 2 years

but i don't think its worth the effort, as SWT already offers checks to find non disposed Resources - but only few people care about.

[HannesWell]

There are several ways to accomplish something similar for example one can have a static method in GC class like this:

public static void localGC(Drawable drawable, SWTRunnable runnable) {
	GC gc = new GC(drawable);
	try {
		runnable.run();
	} finally {
		gc.dispose();
	}
}

I would name it GC.drawOn(drawable, ...) and I think this should be a consumer that accepts a gc, but in general this looks like a good suggestion. But then we should discourage from using the current constructor.

@tobiasmelcher would you be interested in implementing a solution like this?
In order to support arbitrary exceptions being thrown by the lambda, you could consider using the existing SWTCallable and SWTRunnable.