Incorrect assignment assumption in try-with-resources

Answered

Hi team

The following code incorrectly raises a "Value '' is allways 'null'" warning:

Thing thing = null;
try (Resource autoCloseable = new Resource()) { // May throw a ClosingException **ONLY** when closed
thing = buildAThing(autoCloseable); // This throws nothing
return thing;
} catch (ClosingException e) {
return thing; // Warning: 'thing' is always 'null'
}

In this struct (typical case of ignoring closing exceptions), the code block inside try-with-resources never will throw an (checked) exception, and hence, assignment to 'thing' will always occur, and the warning is then false.

 

0
4 comments

Could you please provide full project example for investigation?

0
Avatar
Permanently deleted user

A single-file project does for the warning to rise.

import java.util.stream.Stream;

public class bugcheck {
private static Object nullCheck() {
Long size = null;
try (Stream<Integer> autoCloseable = Stream.of(1, 2, 3, 4, 5)) {
size = autoCloseable.count();
return size;
} catch (Exception e) {
return size;
}
}
}

Here, there is no way 'size' is not assigned: 'Stream.of' rises no exception, so can't fail. Same for 'Stream.count'. So 'size' will allways be assigned.

There are two paths this code may follow:

1.- Happy path:

  1.  'size' is initialized to 'null'
  2.  'autoCloseable' is initialized to the stream of numbers.
  3. `size`is set to the count of elements in 'autoCloseable'
  4. try-with-resources finalizes, invoking 'autoCloseable.close` (no exception thrown)
  5. `nullCheck`returns the value of `size`

2.- Unhappy path:

  1.  'size' is initialized to 'null'
  2.  'autoCloseable' is initialized to the stream of numbers.
  3. `size`is set to the count of elements in 'autoCloseable'
  4. try-with-resources finalizes, invoking 'autoCloseable.close` (exception thrown)
  5. Exception is captured by try-with-resource's 'catch' block
  6. `nullCheck`returns the value of `size`

As you can see, in both paths `size` is always set (step 3), an hence the warning in step 6 is misleading.

Probably related, if we skip initializing 'size', replacing

Long size = null;

by

Long size;

then 'javac' raises error:

$ javac src/bugcheck.java
src\bugcheck.java:10: error: variable size might not have been initialized
            return size;
                   ^
1 error

despite the same reasoning can be used to confirm 'size' will surely be initialized.

 

0
Avatar
Permanently deleted user

Sorry, I realized the previous example may be misleading.

In the example, 'Exception' may also be thrown by the resource definition ('Stream autoCloseable = ...'). In this case, 'size' could not be initialized, and hence the warning is right.

But the issue is still present even if we are only catching an exception that can only  be thrown while closing resources:

public class bugcheck {
private static class MyAutoCloseable implements AutoCloseable {
@Override public void close() throws MyClosingException {}
Long count() { return 0L; }
private class MyClosingException extends Exception {}
}

private static Object nullCheck() {
Long size = null;
try (MyAutoCloseable autoCloseable = new MyAutoCloseable()) {
size = autoCloseable.count();
return size;
} catch (MyAutoCloseable.MyClosingException e) { // Only can be thrown when closing
return size;
}
}
}

 

0

Thanks for the example! I reported an issue on YouTrack: https://youtrack.jetbrains.com/issue/IDEA-181860

Please start following it to receive automatic notifications from YouTrack when the issue is updated.
See https://intellij-support.jetbrains.com/hc/en-us/articles/207241135-How-to-follow-YouTrack-issues-and-receive-notifications   if you are not familiar with YouTrack.

0

Please sign in to leave a comment.