You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Donal Evans <do...@vmware.com> on 2020/10/13 00:30:04 UTC

Reviews requested for LGTM alert fix PR

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