try-with-resources and null resource

[Joe Darcy]

Rémi Forax wrote:
> On 01/24/2011 09:00 PM, Joe Darcy wrote:
>> Rémi Forax wrote:
>>> On 01/24/2011 08:00 PM, Joe Darcy wrote:
>>>> On 1/22/2011 5:09 AM, Rémi Forax wrote:
>>>>> On 01/21/2011 08:38 PM, Joe Darcy wrote:
>>>>>> Rémi Forax wrote:
>>>>>>> I think try-with-resources should do a null check before 
>>>>>>> entering in the try block.
>>>>>>>
>>>>>>> 4. try(AutoCloseable c = null) {
>>>>>>> 5.     // nothing
>>>>>>> 6. }
>>>>>>>
>>>>>>> $ java TryWithresourceNPE
>>>>>>> Exception in thread "main" java.lang.NullPointerException
>>>>>>>      at TryWithresourceNPE.main(TryWithresourceNPE.java:6)
>>>>>>>
>>>>>>> I got a NPE from the ends of the try block, I think it will be 
>>>>>>> better to detect the case
>>>>>>> before entering in the try block like foreach or switch does.
>>>>>>>
>>>>>>> And with this code:
>>>>>>>
>>>>>>> 5. try(InputStream i = null) {
>>>>>>> 6.       i.available();
>>>>>>> 7. }
>>>>>>>
>>>>>>> I got the exception below which is not understandable if you 
>>>>>>> don't know
>>>>>>> how try-with-resources is translated.
>>>>>>>
>>>>>>> $ java TryWithresourceNPE
>>>>>>> Exception in thread "main" java.lang.NullPointerException
>>>>>>>      at TryWithresourceNPE.main(TryWithresourceNPE.java:6)
>>>>>>>      Suppressed: java.lang.NullPointerException
>>>>>>>          at TryWithresourceNPE.main(TryWithresourceNPE.java:7)
>>>>>>
>>>>>> I'm not too concerned about that stacktrace since the primary 
>>>>>> exception points to the right location.
>>>>>
>>>>> But the suppressed exception occurs in a generated code that the 
>>>>> user don't write.
>>>>
>>>> But that is the whole point of try-with-resources, to allow users 
>>>> to not have to write the messy and error-prone clean-up code.
>>>
>>> I have no problem if the error come from the close() itself,
>>> the stacktrace will be explicit in that case.
>>>
>>> But here it comes because the spec allows try(...) to be used with 
>>> null,
>>> oh no wait! the generated finally block doesn't allow it.
>>
>> I would assign the responsibility differently: because the programmer 
>> uses a null expression to initialize a variable, a 
>> NullPointerException is generated when the variable is used.  If the 
>> null-initialized variable is used inside the statement, there is an 
>> additional suppressed exception because of the guarantee to 
>> implicitly call close on the value when the block is exited.
>
> 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.

>
> 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.

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."

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

-Joe