TempVars acquired but not released

[stephengold]

The NetBeans code inspector reports 12 instances in jme3-core where TempVars.get() is invoked and there's a codepath that does not result in the object being released. Here is a typical example:

jmonkeyengine/jme3-core/src/main/java/com/jme3/bounding/BoundingBox.java

Line 731 in c7ff806

TempVars vars = TempVars.get();

If a path leading to a return statement (such as the one in line 759) is taken, then vars is never released. This constitutes a memory leak.

This is an easy fix for anyone with access to NetBeans. Please fix ALL instances reported by the inspector tool.

[pspeed42]

The finally block will always be executed for anything inside the try. So I can't see how this could leak.

[tonihele]

@pspeed42 is right. That tempvars thing is our own hint. I can take a look on the SDK side why it says so. That particular hint was the hardest to convert to the new module format, maybe I made a bug in the process :)

[pspeed42]

Note that further down in the file, public float distanceToEdge(Vector3f point) { is a legitimate example because it doesn't use a try/finally block.

If the passed in point is null then it will throw an NPE after retrieving the temp vars and then never release it.

All temp vars usage should be following the try/finally pattern that the collideWithRay() method is actually doing correctly.

What's concerning is that Netbeans is flagging issues that aren't really there.

[stephengold]

Paul is right. I put too much faith in the inspector.

[MeFisto94]

Well, as @tonihele said, Netbeans itself wouldn't care about tempvars, that's something we put into the SDK specifically and it probably doesn't know about exception handling

[pspeed42]

It would be nice if the tool could be fixed.

...but also note that we accidentally found a legitimate issue because I happened to scan down the file and do a "Paul code analysis". lol

[tonihele]

We will try! We accidentally found two issues, in two projects. So in all I guess this was a success story!
jMonkeyEngine/sdk#291

[tonihele]

@pspeed42 or someone, is there any reason why TempVars can't implement AutoCloseable? You could use these with try-with and all the code analyzers will warn about resource leaks...

[pspeed42]

AutoCloseable is probably fine.... it's just unfortunate that we'd have to implement a 'close' method that language-wise doesn't make as much sense here.

It would be interesting to see if those code analyzers suffer from the same try/finally issue. I would hope not.