Even better, no need for the “catch/throw” chunk, because the method declares 
those caught exceptions:


public AudioInputStream getAudioInputStream(File file)
        throws UnsupportedAudioFileException, IOException {

    final FileInputStream fis = new FileInputStream(file);
    try {
        return getAudioInputStream(new BufferedInputStream(fis));
    } finally {
        fis.close();
    }
}


> On Jan 7, 2015, at 11:41 PM, Dan Rollo <danro...@gmail.com> wrote:
> 
> If this approach is taken, I’d like to suggest using a ‘final’ var instead of 
> ‘null init/null check’, for example:
> 
> public AudioInputStream getAudioInputStream(File file)
>         throws UnsupportedAudioFileException, IOException {
> 
>     final FileInputStream fis = new FileInputStream(file);
>     try {
>         return getAudioInputStream(new BufferedInputStream(fis));
>     } catch(IOException|UnsupportedAudioFileException e) {
>         throw e;
>     } finally {
>         fis.close();
>     }
> }
> 
> 
>> On Jan 7, 2015, at 10:51 PM, Sergey Bylokhov <sergey.bylok...@oracle.com 
>> <mailto:sergey.bylok...@oracle.com>> wrote:
>> 
>> On 08.01.2015 1:13, Phil Race wrote:
>>> Its not clear to me if the bug description is implying an exception was 
>>> thrown
>> UnsupportedAudioFileException is thrown if the URL/File does not point to 
>> valid audio file data recognized by the specific reader, so AudioSystem will 
>> try to move to the next reader and a leak will occur.
>> Actually most of our readers are affected.
>>> Still, something like what you suggest seems to be needed. 
>> right.
>>> 
>>> The owner of this bug is out until next week so I'll let him comment 
>>> further 
>>> after his return. 
>>> 
>>> -phil. 
>>> 
>>> On 01/07/2015 12:42 PM, Mike Clark wrote: 
>>>> Hello all, 
>>>> 
>>>> I wanted to post this as a comment on 
>>>> https://bugs.openjdk.java.net/browse/JDK-8013586 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8013586>, but apparently getting 
>>>> comment access to that system is a bit of a hurdle.  Anyway.  What follows 
>>>> is, I believe, a fix for the aforementioned bug: 
>>>> 
>>>> There is a file handle leak in some of the subclasses of 
>>>> javax.sound.sampled.spi.AudioFileReader, such as 
>>>> com.sun.media.sound.WaveFloatFileReader. 
>>>> 
>>>> Consider com.sun.media.sound.WaveFloatFileReader's method: 
>>>> 
>>>> public AudioInputStream getAudioInputStream(File file) 
>>>>         throws UnsupportedAudioFileException, IOException { 
>>>>     return getAudioInputStream( 
>>>>         new BufferedInputStream(new FileInputStream(file))); 
>>>> } 
>>>> 
>>>> See how there is no attempt to close the FileInputStream if an exception 
>>>> is thrown?  A file handle will remain open on the file until garbage 
>>>> collection is run. Since garbage collection may never run, the file handle 
>>>> may remain open until the JVM exits. And on Windows the open file handle 
>>>> prevents the file from being deleted, which is problematic. 
>>>> 
>>>> Could we fix it by adding a try/catch block? 
>>>> 
>>>> public AudioInputStream getAudioInputStream(File file) 
>>>>         throws UnsupportedAudioFileException, IOException { 
>>>>     FileInputStream fis = null; 
>>>>     try { 
>>>>         fis = new FileInputStream(file); 
>>>>         return getAudioInputStream(new BufferedInputStream(fis)); 
>>>>     } catch(IOException|UnsupportedAudioFileException e) { 
>>>>         if (fis != null) { 
>>>>             fis.close(); 
>>>>         } 
>>>>         throw e; 
>>>>     } 
>>>> } 
>>>> 
>>>> These AudioFileReader subclass methods are usually called by 
>>>> javax.sound.sampled.AudioSystem.getAudioInputStream(File), which calls 
>>>> getAudioInputStream(File) on all registered subclasses of AudioFileReader. 
>>>>  As such, all subclasses of AudioFileReader in the JRE should be reviewed 
>>>> for this problem. 
>>>> 
>>>> best regards, 
>>>> -Mike 
>>>> 
>>> 
>> 
>> 
>> -- 
>> Best regards, Sergey. 
> 

Reply via email to