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