Code review request: 7059709, java/jsse, close the IO in a final block

[Stuart Marks]

On 6/28/11 11:46 AM, Sean Mullan wrote:
> On 6/27/11 9:29 PM, Xuelei Fan wrote:
>> webrev: http://cr.openjdk.java.net/~xuelei/7059709/webrev.00/
>
> It seems you could restructure this code by moving the instantiation of the
> FileInputStream down just before the KeyStore.load and use a try-with-resources
> block instead.

I had thought of this too but using try-with-resources for either of these 
cases isn't obvious.

One difficulty is that the FileInputStreams are only initialized if some 
condition is true, otherwise they're left null. That is, simplified,

     FileInputStream fis = null;
     if (...condition...) {
         fis = ...initialization...
     }
     // process fis if non-null

To use try-with-resources, you'd have to do something like put the conditional 
within the initialization expression. The only way to do this in-line in Java 
is to use the ternary (?:) operator, like so:

     try (FileInputStream fis = ...condition... ? ...initialization... : null) {
         // process fis if non-null
     }

or the conditional could be evaluated prior to the try, with the resource 
variable being an alias:

     FileInputStream fis = null;
     if (...condition...) {
         fis = ...initialization...
     }
     try (FileInputStream fis2 = fis) {
         // process fis2 if non-null
     }

or even by refactoring the conditional initialization into a separate method:

     try (FileInputStream fis = openFileOrReturnNull(...)) {
         // process fis if non-null
     }

Another approach would be to call KeyStore.load() from the different condition 
arms, instead of conditionally initializing a variable:

     if (...condition...) {
         try (FileInputStream fis = ...initialization...) {
             ks.load(fis, passwd);
         }
     } else {
         ks.load(null, passwd);
     }

This initializes the input streams closer to where they're used, but this might 
change the behavior if an error occurs while opening the stream; additional 
initializations or even side effects might occur before the error is reached. I 
haven't inspected the code thoroughly to determine whether this is the case. It 
also puts big initialization expression into the t-w-r, which might be ugly.

Now, as you might know, I'm a big fan of try-with-resources, but using it here 
brings in a potentially large restructuring. This might or might not be 
warranted for this particular change; I don't know the tradeoffs involved in 
this code.

s'marks