Code Review 7000511: PrintStream, PrintWriter, Formatter leave files open when exception thrown

[Chris Hegarty]

Hi Dave,

> In src/share/classes/java/util/Formatter.java, shouldn't lines 1977
> and 2084 also use wrapFileOutputStream?

No, I don't believe it is necessary to use wrapFileOutputStream for 
these since the OutputStreamWriter constructor can never fail here 
(therefore we will never need to close the file stream).

The OutputStreamWrite constructor can only fail ( in this cases ) when 
null is passed as the charset name, or an unsupported charset name is 
paseed.

I am going to rework the fix as per Alan's comments [1]. Instead of 
checking that the OutputStreamWriter has been successfully constructed 
we can verify the passed arguments before creating the file stream.

Thanks,
-Chris.

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-December/005453.html

> 1975     public Formatter(String fileName) throws FileNotFoundException {
> 1976         this(Locale.getDefault(Locale.Category.FORMAT),
> 1977              wrapFileOutputStream(new FileOutputStream(fileName)));
> 1978     }
>
> 2082     public Formatter(File file) throws FileNotFoundException {
> 2083         this(Locale.getDefault(Locale.Category.FORMAT),
> 2084              wrapFileOutputStream(new FileOutputStream(file)));
> 2085     }
>
> - Dave
>
> On Tue, Dec 14, 2010 at 10:07 AM, Chris Hegarty
> <chris.hegarty at oracle.com>  wrote:
>> Failing java.io.PrintStream, java.io.PrintWriter, java.util.Scanner, and
>> java.util.Formatter multi-arg constructors that take a java.io.File or
>> String filename (as well as one or more additional args) opens a
>> FileIn/OutputStream to the given File/filename. If one of the other given
>> args causes the constructor to fail ( null or unsupported charset for
>> example ) the FileIn/OutputStream is never closed, and the application does
>> not have a reference to it. You rely on the finalizer to close the stream.
>>
>> This is most serious on Windows because you cannot remove a file if there is
>> an open handle to it.
>>
>> I also cleaned up an existing regression test that fails in samevm mode
>> (partly) because of this. And removed another excluded test from the list
>> since its bug was fixed some time ago.
>>
>> Webrev:
>>   http://cr.openjdk.java.net/~chegar/7000511/webrev.00/webrev/
>>
>> -Chris.
>>