Bug Code cleanup in Parser (fixes a memory leak)

Catch-22

Active member
Some general code clean up in Parser.java.

Fixes a memory leak with ostream as we were not closing the stream beforehand.
Disambiguates the hashing method used in parseRecord for anonymous record type names.
Generic type safety fixes.
 

Attachments

  • parser_cleanup.patch
    3.1 KB · Views: 42

Catch-22

Active member
Serious question, how is that an improvement?

array.hashCode() is ambiguous in that it can be confused as Arrays.hashCode(array) which does a different thing (it's "new" in 1.5). Most of the time when people do array.hashCode() they mean to do Arrays.hashCode(array), which is what I thought at first, but on closer inspection in that context it's not necessary to hash the contents of the array and it's just being used as a way of uniquely identifying the array itself.

Short answer, array.hashCode() and System.identityHashCode(array) will compile to the same thing provided you don't override the hashCode() method, but using System.identityHashCode shows that it's what you definitely intended to do.
 
Last edited:

lostcalpolydude

Developer
Staff member
Closing a ByteArrayOutputStream does nothing, apparently. It looks like garbage collection should clean up that object after parseCommand() returns.
 

Catch-22

Active member
Closing a ByteArrayOutputStream does nothing, apparently. It looks like garbage collection should clean up that object after parseCommand() returns.

Most JVM garbage collection is non-deterministic. My understanding is that just because an object goes out of scope, does not mean it will get garbage collected. Regardless of that, I see in the documentation that the close() method "does nothing".

I was interested enough to do some googling, as my understanding of best practices says that all streams should be closed when you're finished with them. Here's what my searching came up with, feel free to read for yourself and make the decision as you see fit.

Also I should note that the close() method should probably be part of a finally block, which is not the case in the patch I provided.
 
Top