I don't think this is necessary, since closeQuietly just swallows the
IOExceptions that might get thrown during close and logs a warning. It
never propagates the exception. Consecutive closeQuietly calls should
therefore always get executed even without finally block.

BTW, it seems that

Closeables.closeQuietly

was announced deprecated in guava 14. So I guess at some point all
this should get moved into a "try with resource block" supported in
jdk 1.7.



On Fri, Dec 20, 2013 at 11:49 AM, James Taylor <notifications@github.com>wrote:

> In src/main/java/com/salesforce/phoenix/cache/aggcache/SpillFile.java:
>
> > @@ -102,14 +100,20 @@ public int getMaxPageId() {
> >      }
> >
> >      @Override
> > -    public void close() throws IOException {
> > +    public void close() {
> > +        // Swallow IOException
> >          Closeables.closeQuietly(fc);
>
> Do you think it would make sense to nest these like this?
>
> try {
>     Closeables.closeQuietly(fc);
> } finally {
>     try {
>         Closeables.closeQuietly(file);
>     } finally {
>         try {
>             tempFile.delete();
>         } catch (SecurityException e) {
>                 logger.warn("IOException thrown while closing Closeable." + e);
>         } finally {
>             tempFile = null;
>         }
>     }
> }
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/forcedotcom/phoenix/pull/650/files#r8508839>
> .
>