1

Is it safe to use Java's try with resources in Android- does it check if the closeable is not null and does it catch exceptions thrown by close while trying to close it?

if I convert this:

 try { inChannel.transferTo(0, inChannel.size(), outChannel); } finally { if (inChannel != null) { inChannel.close(); } if (outChannel != null) { outChannel.close(); } }

to

 try (FileChannel inChannel = new FileInputStream(src).getChannel(); FileChannel outChannel = new FileOutputStream(dst).getChannel()) { inChannel.transferTo(0, inChannel.size(), outChannel); }

will it check if inChannel and outChannel are not null before trying to call close?

Also, is it safe to use try with resources here:

 try { cursor = context.getContentResolver().query( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, new String[]{MediaStore.Images.Media.DATA}, MediaStore.Images.Media._ID + " =? ", new String[]{"" + imageIdInMediaStore}, null); if (cursor != null && cursor.getCount() > 0) { cursor.moveToFirst(); return cursor.getString(0); } else { return ""; } } catch (Exception e) { return ""; } finally { if (cursor != null && !cursor.isClosed()) { cursor.close(); cursor = null; } }

The finally block does an important check for !cursor.isClosed() - will try with resources figure out how to do that, or should I leave this unchanged?

Improve this question
3
  • 1
    stackoverflow.com/questions/23611940/… Commented Oct 30, 2017 at 16:04
  • Why do you say the cursor.isClosed() is "an important check"? Just call cursor.close(). The close() method is required to be idempotent, so the isClosed() call is redundant. Calling close() on an already closed cursor does nothing. As the javadoc says: If the stream is already closed then invoking this method has no effect. Commented Oct 30, 2017 at 16:29
  • Why would they be null? How could they be null? Commented Oct 30, 2017 at 16:36

1 Answer 1

Reset to default
4

The easiest way to find that out yourself is writing a test class simply telling you about it:

import junit.framework.TestCase; public class __Test_AutoClosable extends TestCase { public void testWithNull() throws Exception { try (TestClosable tc = getNull()) { assertNull("check existance of closable", tc); } } public void testNonNullWithoutException() throws Exception { try (TestClosable tc = getNotNull(false)) { assertNotNull("check existance of closable", tc); } } public void testNonNullWithException() throws Exception { try (TestClosable tc = getNotNull(true)) { assertNotNull("check existance of closable", tc); } catch(Exception e) { assertEquals("check message", "Dummy Exception", e.getMessage()); } } TestClosable getNull() { return null; } TestClosable getNotNull(boolean throwException) { return new TestClosable(throwException); } static class TestClosable implements AutoCloseable { private boolean throwException; TestClosable(boolean throwException) { this.throwException = throwException; } @Override public void close() throws Exception { if (throwException) { throw new Exception("Dummy Exception"); } } } }

This class runs through without errors, so the answers to your questions are:

  • Yes, null as response is possible, it is checked in the close-phase
  • Exceptions thrown on close are not catched but must be catched and handled by yourself

If you think about this a bit, that makes complete sense. Not supporting null would draw the whole construct useless, simply ignoring closes, even if they are implicitly done, is a bad, bad thing.

Edit: The testcase above is the result but for a "real" test, e.g. being added to your source base, you should actually check if the close-method in the autoclosable has been called.

Concerning your second question: If there are cases where you don't want the close to happen, a 1:1-change to try-with-resources doesn't work but you can do something like this:

public class CursorClosable implements AutoClosable { private Cursor cursor; public CursorClosable(Cursor cursor) { this.cursor = cursor; } public Cursor getCursor() { return cursor; } @Override public void close() throws Exception { if (cursor != null && !cursor.isClosed()) { cursor.close(); } } }

Your new code would then look like this:

try (Cursor cursorac = new CursorClosable(context.getContentResolver().query( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, new String[]{MediaStore.Images.Media.DATA}, MediaStore.Images.Media._ID + " =? ", new String[]{"" + imageIdInMediaStore}, null)) { Cursor cursor = cursorac.getCursor(); if (cursor != null && cursor.getCount() > 0) { cursor.moveToFirst(); return cursor.getString(0); } else { return ""; } } catch (Exception e) { return ""; }

I'm not sure if the check for isClosed is really necessary so maybe you don't need this kind of "hack" for this particular example but it's still valid for other examples where you don't want to close resources.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

However, if the try block throws an exception, and then the close call throws an exception too, the close exception doesn't override/replace the first exception, but gets attached to the first exception as a suppressed exception. See The Java™ Tutorials - The try-with-resources Statement - Suppressed Exceptions.
Thank you. So we cannot benefit from the brevity if a try with resources if the close method could throw an exception OR if there are additional checks that the try with resources doesn't know about (for instance if !cursor.isClosed()?
@K.R. Sure you can benefit, as long as the resource object implements Closeable or AutoCloseable. Calls like isClosed() is unnecessary, since the try-with-resources block should be the only one closing the object anyway. Besides, the close method is supposed to be idempotent, meaning that repeated calls should be ignored.
@Lothar In the updated answer, with CursorClosable, the cursor variable cannot be null. All the code inside the block must change cursor to cursor.getCursor(). Adding CursorClosable shouldn't be necessary, since the isClosed() call is redundant, given that close() is required to be idempotent.
@idempotent I changed the name of the result in the try-with-resource-block to some other name and call getCursor in the actual block to fix one issue. The other point is valid but I tried to keep the code of the original question as unchanged as possible to be able to see what changes and what keeps the same (but later mentioned that the actual example isn't a good one).

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.