<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">It’s great to see activity on this.</div><div class=""><br class=""></div>Any chance of adding some unit tests of the changes? (Could stream mocks help?)<div class="">This code and related “.close()” logic has be a challenge for a long time. Might be worth tweaking to make it more testable now (even if that involved something like a new package visible method that takes an easily mocked stream object - instead of a file…). </div><div class=""><br class=""></div><div class="">Dan</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 16, 2015, at 3:26 PM, Sergey Bylokhov <<a href="mailto:Sergey.Bylokhov@oracle.com" class="">Sergey.Bylokhov@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Hi, Everybody.</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On 10.01.15 1:09, Florian Bomers wrote:</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">At the time, I've fixed the same type of bug in WaveFileReader and the<br class="">likes. It was tracked under bug #4325421 and I *should* have written a<br class="">unit test. If indeed,  you should find it by looking for a unit test<br class="">with that bug id.<br class=""><br class="">In WaveFileReader, I fixed it without a clumsy catch clause --<br class="">analogous to this:<br class=""><br class="">  FileInputStream fis = new FileInputStream(file);<br class="">  BufferedInputStream bis = new BufferedInputStream(fis);<br class="">  AudioInputStream ais = null;<br class="">  try {<br class="">    ais = getAudioInputStream(bis);<br class="">  } finally {<br class="">    if (ais == null) {<br class="">      bis.close();<br class="">    }<br class="">  }<br class="">  return ais;<br class=""></blockquote><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">In this example if getAudioInputStream() will throw UnsupportedAudioFileException and the last close() method will throw IOException, then we will not check the next audio reader.</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I have a small prototype where I merge all implementations of getAudioXXX to the SunFileReader. It will fix this and related issues:</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="http://cr.openjdk.java.net/~serb/8013586/webrev.01" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">http://cr.openjdk.java.net/~serb/8013586/webrev.01</a><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Issues which were fixed:</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">- Streams were not closed when necessary. In the fix they are closed always in case of any exceptions. JDK-8013586</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">- Some of the readers do not reset the streams when they throw an UnsupportedAudioFileException exception. JDK-8130305</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">- Some of the readers(like AiffFileReader) do not wrap (FileInputStream, etc) in BufferedInputStream.</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I assume that this fix should not cause regressions so I will send a review request for it as is, after the testing. I found some other small issues(like handling EOFException) in this code but will fix it separately later.</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Also I have a related question about WaveExtensibleFileReader why it was not added to the spi.AudioFileReader? Because of this WaveExtensibleFileReader actually is never used.</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Best, Florian On 09.01.2015 20:21, Dan Rollo wrote:</span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class="">Yikes, Good point Klaus! Forgot the caller wants to actually use a<br class="">valid stream for the non-exceptional case. Would have to move the<br class="">is.close() back into a catch clause. I’ll try to post a better one<br class="">later. (Any unit tests of this sort of thing exist in the tree now? -<br class="">if not, I could try a unit test too).<br class=""><br class=""><blockquote type="cite" class="">On Jan 9, 2015, at 12:34 PM, Klaus Jaensch<br class=""><<a href="mailto:klausj@phonetik.uni-muenchen.de" class="">klausj@phonetik.uni-muenchen.de</a><br class=""><<a href="mailto:klausj@phonetik.uni-muenchen.de" class="">mailto:klausj@phonetik.uni-muenchen.de</a>>> wrote:<br class=""><br class="">Hi Dan,<br class=""><br class="">Am 09.01.2015 um 05:37 schrieb Dan Rollo:<br class=""><blockquote type="cite" class="">Even better, no need for the “catch/throw” chunk, because the<br class="">method declares those caught exceptions:<br class=""><br class=""><br class="">public AudioInputStream getAudioInputStream(File file)<br class="">        throws UnsupportedAudioFileException, IOException {<br class=""><br class="">    final FileInputStream fis = new FileInputStream(file);<br class="">    try {<br class="">        return getAudioInputStream(new BufferedInputStream(fis));<br class="">    } finally {<br class="">        fis.close();<br class="">    }<br class="">}<br class=""></blockquote>I think your code will not work. If the call to<br class="">getAudioInputStream(InputStream) succeeds the code always closes the<br class="">stream before it is returned.<br class=""><br class=""><br class="">I suggest this approach:<br class=""><br class="">public AudioInputStream getAudioInputStream(File file)<br class="">            throws UnsupportedAudioFileException, IOException {<br class="">        FileInputStream fis=new FileInputStream(file);<br class="">        BufferedInputStream bis=new BufferedInputStream(fis);<br class="">        AudioInputStream ais=null;<br class="">        try{<br class="">            ais=getAudioInputStream(bis);<br class="">        } catch(IOException|UnsupportedAudioFileException e) {<br class="">            if(bis!=null){<br class="">                bis.close();<br class="">            }<br class="">            throw e;<br class="">        }<br class="">        return ais;<br class="">    }<br class=""><br class="">Closes the BufferedInputStream as well as the underlying<br class="">FileInputStream.<br class=""><br class="">I think the method:<br class="">public AudioInputStream getAudioInputStream(URL url)<br class=""><br class="">needs to be changed in the same way.<br class=""><br class=""><br class="">Best regards,<br class="">Klaus<br class=""><br class=""><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" class="">On Jan 7, 2015, at 11:41 PM, Dan Rollo <<a href="mailto:danrollo@gmail.com" class="">danrollo@gmail.com</a><br class=""><<a href="mailto:danrollo@gmail.com" class="">mailto:danrollo@gmail.com</a>>> wrote:<br class=""><br class="">If this approach is taken, I’d like to suggest using a ‘final’ var<br class="">instead of ‘null init/null check’, for example:<br class=""><br class="">public AudioInputStream getAudioInputStream(File file)<br class="">        throws UnsupportedAudioFileException, IOException {<br class=""><br class="">    final FileInputStream fis = new FileInputStream(file);<br class="">    try {<br class="">        return getAudioInputStream(new BufferedInputStream(fis));<br class="">    } catch(IOException|UnsupportedAudioFileException e) {<br class="">        throw e;<br class="">    } finally {<br class="">        fis.close();<br class="">    }<br class="">}<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Jan 7, 2015, at 10:51 PM, Sergey Bylokhov<br class=""><<a href="mailto:Sergey.Bylokhov@oracle.com" class="">Sergey.Bylokhov@oracle.com</a> <<a href="mailto:Sergey.Bylokhov@oracle.com" class="">mailto:Sergey.Bylokhov@oracle.com</a>>><br class="">wrote:<br class=""><br class="">On 08.01.2015 1:13, Phil Race wrote:<br class=""><blockquote type="cite" class="">Its not clear to me if the bug description is implying an<br class="">exception was thrown<br class=""></blockquote>UnsupportedAudioFileException is thrown if the URL/File does not<br class="">point to valid audio file data recognized by the specific reader,<br class="">so AudioSystem will try to move to the next reader and a leak<br class="">will occur.<br class="">Actually most of our readers are affected.<br class=""><blockquote type="cite" class="">Still, something like what you suggest seems to be needed.<br class=""></blockquote>right.<br class=""><blockquote type="cite" class="">The owner of this bug is out until next week so I'll let him<br class="">comment further<br class="">after his return.<br class=""><br class="">-phil.<br class=""><br class="">On 01/07/2015 12:42 PM, Mike Clark wrote:<br class=""><blockquote type="cite" class="">Hello all,<br class=""><br class="">I wanted to post this as a comment<br class="">on <a href="https://bugs.openjdk.java.net/browse/JDK-8013586" class="">https://bugs.openjdk.java.net/browse/JDK-8013586</a>, but<br class="">apparently getting comment access to that system is a bit of a<br class="">hurdle.  Anyway.  What follows is, I believe, a fix for the<br class="">aforementioned bug:<br class=""><br class="">There is a file handle leak in some of the subclasses of<br class="">javax.sound.sampled.spi.AudioFileReader, such as<br class="">com.sun.media.sound.WaveFloatFileReader.<br class=""><br class="">Consider com.sun.media.sound.WaveFloatFileReader's method:<br class=""><br class="">public AudioInputStream getAudioInputStream(File file)<br class="">        throws UnsupportedAudioFileException, IOException {<br class="">    return getAudioInputStream(<br class="">        new BufferedInputStream(new FileInputStream(file)));<br class="">}<br class=""><br class="">See how there is no attempt to close the FileInputStream if an<br class="">exception is thrown?  A file handle will remain open on the<br class="">file until garbage collection is run. Since garbage collection<br class="">may never run, the file handle may remain open until the JVM<br class="">exits. And on Windows the open file handle prevents the file<br class="">from being deleted, which is problematic.<br class=""><br class="">Could we fix it by adding a try/catch block?<br class=""><br class="">public AudioInputStream getAudioInputStream(File file)<br class="">        throws UnsupportedAudioFileException, IOException {<br class="">    FileInputStream fis = null;<br class="">    try {<br class="">        fis = new FileInputStream(file);<br class="">        return getAudioInputStream(new BufferedInputStream(fis));<br class="">    } catch(IOException|UnsupportedAudioFileException e) {<br class="">        if (fis != null) {<br class="">            fis.close();<br class="">        }<br class="">        throw e;<br class="">    }<br class="">}<br class=""><br class="">These AudioFileReader subclass methods are usually called by<br class="">javax.sound.sampled.AudioSystem.getAudioInputStream(File),<br class="">which calls getAudioInputStream(File) on all registered<br class="">subclasses of AudioFileReader.  As such, all subclasses of<br class="">AudioFileReader in the JRE should be reviewed for this problem.<br class=""><br class="">best regards,<br class="">-Mike<br class=""><br class=""></blockquote></blockquote><br class="">--<span class="Apple-converted-space"> </span><br class="">Best regards, Sergey.<br class=""></blockquote></blockquote></blockquote><br class="">--<span class="Apple-converted-space"> </span><br class="">------------------------------------------<br class="">Klaus Jaensch<br class="">Muenchen<br class="">Germany<br class=""><br class="">Institut fuer Phonetik und Sprachverarbeitung<br class="">Schellingstr.3/II<br class="">Room 223 VG<br class="">80799 München<br class=""><br class="">Phone (Work): +49-(0)89-2180-2806<br class="">Fax:          +49-(0)89-2180-5790<br class="">EMail: <a href="mailto:klausj@phonetik.uni-muenchen.de" class="">klausj@phonetik.uni-muenchen.de</a><br class=""></blockquote></blockquote></blockquote><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Best regards, Sergey.</span></div></blockquote></div><br class=""></div></body></html>