At the time, I've fixed the same type of bug in WaveFileReader and the
likes. It was tracked under bug #4325421 and I *should* have written a
unit test. If indeed, you should find it by looking for a unit test
with that bug id.
In WaveFileReader, I fixed it without a clumsy catch clause --
analogous to this:
FileInputStream fis = new FileInputStream(file);
BufferedInputStream bis = new BufferedInputStream(fis);
AudioInputStream ais = null;
try {
ais = getAudioInputStream(bis);
} finally {
if (ais == null) {
bis.close();
}
}
return ais;
Best,
Florian
On 09.01.2015 20:21, Dan Rollo wrote:
> Yikes, Good point Klaus! Forgot the caller wants to actually use a
> valid stream for the non-exceptional case. Would have to move the
> is.close() back into a catch clause. I’ll try to post a better one
> later. (Any unit tests of this sort of thing exist in the tree now? -
> if not, I could try a unit test too).
>
>> On Jan 9, 2015, at 12:34 PM, Klaus Jaensch
>> <kla...@phonetik.uni-muenchen.de
>> <mailto:kla...@phonetik.uni-muenchen.de>> wrote:
>>
>> Hi Dan,
>>
>> Am 09.01.2015 um 05:37 schrieb Dan Rollo:
>>> 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();
>>> }
>>> }
>>
>> I think your code will not work. If the call to
>> getAudioInputStream(InputStream) succeeds the code always closes the
>> stream before it is returned.
>>
>>
>> I suggest this approach:
>>
>> public AudioInputStream getAudioInputStream(File file)
>> throws UnsupportedAudioFileException, IOException {
>> FileInputStream fis=new FileInputStream(file);
>> BufferedInputStream bis=new BufferedInputStream(fis);
>> AudioInputStream ais=null;
>> try{
>> ais=getAudioInputStream(bis);
>> } catch(IOException|UnsupportedAudioFileException e) {
>> if(bis!=null){
>> bis.close();
>> }
>> throw e;
>> }
>> return ais;
>> }
>>
>> Closes the BufferedInputStream as well as the underlying
>> FileInputStream.
>>
>> I think the method:
>> public AudioInputStream getAudioInputStream(URL url)
>>
>> needs to be changed in the same way.
>>
>>
>> Best regards,
>> Klaus
>>
>>
>>>
>>>
>>>> On Jan 7, 2015, at 11:41 PM, Dan Rollo <danro...@gmail.com
>>>> <mailto: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, 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.
>>>>
>>>
>>
>>
>> --
>> ------------------------------------------
>> Klaus Jaensch
>> Muenchen
>> Germany
>>
>> Institut fuer Phonetik und Sprachverarbeitung
>> Schellingstr.3/II
>> Room 223 VG
>> 80799 München
>>
>> Phone (Work): +49-(0)89-2180-2806
>> Fax: +49-(0)89-2180-5790
>> EMail: kla...@phonetik.uni-muenchen.de
>
--
Florian Bomers
Bome Software
everything sounds.
http://www.bome.com
__________________________________________________________________
Bome Software GmbH & Co KG Gesellschafterin:
Dachauer Str.187 Bome Komplementär GmbH
80637 München, Germany Geschäftsführung: Florian Bömers
Amtsgericht München HRA95502 Amtsgericht München HRB185574
