Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Free or Close #186

Closed
ShadowLordAlpha opened this issue Apr 22, 2016 · 6 comments
Closed

Free or Close #186

ShadowLordAlpha opened this issue Apr 22, 2016 · 6 comments

Comments

@ShadowLordAlpha
Copy link

Some of the different classes have a method called free()that cleans up the native resources that an object uses. In c this makes sense as that memory is being freed up but in java this method is normally known as close(). Method functionality would not change but some IDEs will warn a developer when they do not close a possible object and if implementing AutoCloseable it becomes possible to use these objects in a try with resource instead of a try finally or simply closing it later and possibly missing something upon an error.

It could also be done by making a simple interface called that could look like this

public interface Freeable extends AutoCloseable {
 default void close() {
  free();
 }

 void free();
}

That would keep method names as they are but allow use of the try with resource and possible ide warnings.

@Spasi
Copy link
Member

Spasi commented Apr 22, 2016

Good idea, thanks!

@Spasi
Copy link
Member

Spasi commented Apr 23, 2016

Apparently this change is problematic in IDEs that have try-with-resources warnings enabled.

Specifically, a lot of times a NativeResource instance does not own the associated native memory (example: GLFWVidMode vidmode = glfwGetVideoMode(monitor)), so freeing/closing it is not only unnecessary, but also illegal (would crash the JVM). But IDE inspections do not have that information and report warnings in many situations where they shouldn't.

In IntelliJ IDEA, the relevant inspection is Java -> Resource management issues -> AutoCloseable used without 'try'-with-resources. If you'd like to keep it enabled, you may add org.lwjgl.system.NativeResource under Options -> Ignored AutoCloseable resource types and the problem is fixed.

Afaik Eclipse does not offer such customization. The following warnings should be disabled:

  • Code style -> Resource not managed via try-with-resources (1.7 or higher)
  • Potential programming problems -> Potential resource leak

Please post below if you have information on NetBeans.

@Guichaguri
Copy link

Sorry, but I think this is being more problematic than useful. I think this is the first Java 8 functionality in LWJGL 3, and I think it should be removed to keep it compatible with Java 7, since a lot of users still use Java 7.
The only thing I can suggest is renaming free to close or implementing both of them without default methods :/

@Spasi
Copy link
Member

Spasi commented May 4, 2016

See #177, LWJGL 3 requires Java 8.

@huhlig
Copy link

huhlig commented May 4, 2016

@Guichaguri Just to note, Plumbr is a server performance monitoring application and its numbers arn't really reflective of the desktop market.

@Guichaguri
Copy link

No problem. At least I still have a Swing alternative for older Java versions, and I can always go back to LWJGL 2. Thanks.

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

No branches or pull requests

4 participants