try-with-resources and null resource

[Rémi Forax]

On 01/25/2011 03:16 AM, Joe Darcy wrote:

[...]

>>
>> The main problem is what if a try-with-resources have a body that 
>> don't use the variable.
>> Let suppose I design this interface:
>>
>> interface Transaction extends AutoCloseable {
>>   public void close() throw TransactionException;
>> }
>>
>> I want to use it with a try-with-resources to use its marvellous 
>> suppressed exceptions management:
>>
>> try(Transaction t = Transactions.getTransaction()) {
>>   // I don't use 't' here
>> }
>
>
> Javac will warn you about this situation.

Why ? It's a fair code.
And adding a new warning has a cost. If there is too much 
not-that-useful warnings,
users will don't care about them or worst deactivate them all.
If there is a warning, what is the way to fix it ? Introduce a 
t.getClass() ?

try(Transaction t = Transactions.getTransaction()) {
    t.getClass(); // remove that stupid javac warning
}

That's exactly what I'm proposing :)

>
>>
>> here if the transaction is null, it will execute the body and then 
>> throws a NPE as a sidekick.
>
> Yes, that is what the specification and implementation currently require.
>
>> I don't think it's the behaviour we want.
>>
>>>
>>>>
>>>> If you take a look to how try/finally are coded now, you will find 
>>>> a nullcheck
>>>> before the try or in the finally block.
>>>>
>>>>>
>>>>>> I don't like the idea that a user mistake blow up in a generated 
>>>>>> code,
>>>>>> try-with-resources should protect itself.
>>>>>
>>>>> I think this stack trace can easily be explained to programmers by 
>>>>> saying "the suppressed exception comes from the close call you did 
>>>>> have to write yourself."
>>>>
>>>> You don't have to explain something which is not surprising.
>>>
>>> New features should be expected to require nonzero explanation 
>>> simply by virtue of being new.
>>
>> I have no problem to teach why try-with-resources was introduced or 
>> when to use it.
>> I just don't want to explain why try-with-resources doesn't behave 
>> like all other Java constructs.
>> They all works in a similar way:
>>   synchronized(null) { }, for(Object o: null) { }, switch(null) { }
>> why try(Object o = null) {} should have a different semantics ?
>>
>
> From a certain point of view, try with resource does have the same 
> semantics, you get the NPE from the location at which you go do 
> something with the null value.  If you don't use the null resource 
> variable inside the try-with-resources block, you'll get the NPE in 
> the compiler-generated call to close.  If you use the null value 
> inside the try-with-resources block, you'll get the first NPE in the 
> block.

As Reinier said, I don't want to think about how the code is translated 
when I use a feature of a language.
In my opinion, if you need to know how a feature is translated to use 
it, it's a design failure.
Explaining bug 4030374 before it was fixed was a painful teaching moment.

>
> I'll consider adding an explicit null check in the generated before 
> calling close to generate a better exception message, something like
>
>    "Attempt to call close on null resource " + r.name())
>
> or
>
>    "Attempt to call close on null resource at position " + i + " of " 
> + n " resources."

That's better than nothing.
But I don't like this solution because of the way the compiler translate 
try/finally.
This check will appear two or more times in the bytecode.

Also note that from the performance point of the doing the nullcheck 
up-front
doesn't introduce any performance penalty because the VM has to do a 
nullcheck when calling close().

>
> However, I don't plan to change the overall semantics of the feature 
> to build in an up-front null-check.

try-with-resources fails to handle correctly nullable resource.
You have no choice :)

>
> -Joe

Rémi