Hi Geode dev,

As part of continued efforts to address alerts generated by LGTM analysis of 
the Geode codebase, I've opened a PR that addresses 108 of the "Potential 
input/output resource leak" alerts currently flagged: 
https://github.com/apache/geode/pull/5582. This encompasses most (but not all, 
see below) of that type of alert in the codebase and reduces the total number 
of LGTM alerts by 20%.

The majority of these alerts were fixed by simply introducing a 
try-with-resources block around the relevant lines, which ensures that 
resources are correctly released in the event of exceptions being thrown. This 
approach is equivalent to, but more concise than, the try/catch/finally style 
of resource handling in which resources are declared prior to the try block, 
assigned in the try block, and released/closed in the finally block.

Some of the alerts flagged by LGTM were false positives or could not use the 
try-with-resources approach, such as these two in ClientSideHandshakeImpl.java: 
https://lgtm.com/projects/g/apache/geode/snapshot/f21e8b26cbfc5af9aca2160811921cba4c46635e/files/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientSideHandshakeImpl.java?sort=name&dir=ASC&mode=heatmap#xbef4ecbd7219d16e:1
 where closing the DataInputSteam or DataOutputStream on method exit would also 
close the parent streams on the socket, rendering the connection unusable. 
False positives such as this, and more complex scenarios which could not be 
addressed with minimal changes are left as future work.

If anyone could spare some time to review the changes, I would much appreciate 
it.

Thanks,
Donal

Reply via email to