commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [fileupload] file stream leaks?
Date Fri, 07 Feb 2014 13:26:59 GMT
Not until the base requirement is Java 7. It's 5 now. 

Gary

-------- Original message --------
From: Javin Paul <savingfunda@gmail.com> 
Date:02/07/2014  07:14  (GMT-05:00) 
To: Commons Developers List <dev@commons.apache.org> 
Subject: Re: [fileupload] file stream leaks? 

Any chance to convert them using try-with-resource statements in future, I
guess that will make code much cleaner.


On Fri, Feb 7, 2014 at 1:02 AM, Gary Gregory <garydgregory@gmail.com> wrote:

> On Thu, Feb 6, 2014 at 11:23 AM, sebb <sebbaz@gmail.com> wrote:
>
> > On 6 February 2014 16:06, Gary Gregory <garydgregory@gmail.com> wrote:
> > > On Thu, Feb 6, 2014 at 10:18 AM, sebb <sebbaz@gmail.com> wrote:
> > >
> > >> On 6 February 2014 15:04, Gary Gregory <garydgregory@gmail.com>
> wrote:
> > >> > On Thu, Feb 6, 2014 at 9:04 AM, sebb <sebbaz@gmail.com> wrote:
> > >> >
> > >> >> On 6 February 2014 13:53, Gary Gregory <garydgregory@gmail.com>
> > wrote:
> > >> >> > Looking at code like DiskFileItem:684:
> > >> >> >
> > >> >> >             FileInputStream input = new
> FileInputStream(dfosFile);
> > >> >> >             IOUtils.copy(input, output);
> > >> >> >
> > >> >> > Why is this not:
> > >> >> >
> > >> >> >             FileInputStream input = new
> FileInputStream(dfosFile);
> > >> >> >             try {
> > >> >> >                 IOUtils.copy(input, output);
> > >> >> >             } finally {
> > >> >> >                 input.close();
> > >> >> >             }
> > >> >> >
> > >> >> > ?
> > >> >>
> > >> >> Oversight?
> > >> >>
> > >> >> The local Streams.copy() methods do close input and optionally
> > output.
> > >> >> Maybe it was thought IO did the same?
> > >> >>
> > >> >> The close should perhaps be
> > >> >>
> > >> >> IOUtils.closeQuietly()
> > >> >>
> > >> >> Are we interested in knowibg about input close() failures?
> > >> >>
> > >> >
> > >> > Good question. I would guess 'yes'. If you write to a file and then
> > >> cannot
> > >> > close it, you might not have flushed it, so the file could be
> corrupt?
> > >>
> > >> But this is an _input_ file.
> > >> Are we interested in close failure once the copy has completed?
> > >>
> > >
> > > I would say no. BUT... if you replace the Streams code with:
> > >
> > >     public static long copy(InputStream inputStream, OutputStream
> > > outputStream, boolean closeOutputStream, byte[] buffer)
> > >             throws IOException {
> > >         try {
> > >             return IOUtils.copy(inputStream, outputStream);
> > >         } finally {
> > >             IOUtils.closeQuietly(inputStream);
> > >             if (closeOutputStream) {
> > >                 IOUtils.closeQuietly(outputStream);
> > >             }
> > >         }
> > >     }
> > >
> > > You will get an NPE in  IOUtils.copy(inputStream, outputStream).
> > >
> > > Because in some cases [fileupload] calls Streams.copy with a null
> output
> > > stream. The Javadoc for IOUtils.copy states that this is grounds for an
> > NPE.
> > >
> > > So that's a no-go. I wonder if we should propose to make IOUtils.copy
> > > ignore null output to accommodate this scenario?
> >
> > No.
> > Change of API behaviour and allowing null will probably hide some bugs.
> >
> > However, Streams could potentially call IOUtils.
> >
>
> Streams now reuses IOUtils.closeQuietly().
>
> Gary
>
>
> >
> > > Gary
> > >
> > >>
> > >> > Gary
> > >> >
> > >> >
> > >> >>
> > >> >> > Gary
> > >> >> >
> > >> >> >
> > >> >> > --
> > >> >> > E-Mail: garydgregory@gmail.com | ggregory@apache.org
> > >> >> > Java Persistence with Hibernate, Second Edition<
> > >> >> http://www.manning.com/bauer3/>
> > >> >> > JUnit in Action, Second Edition <
> http://www.manning.com/tahchiev/>
> > >> >> > Spring Batch in Action <http://www.manning.com/templier/>
> > >> >> > Blog: http://garygregory.wordpress.com
> > >> >> > Home: http://garygregory.com/
> > >> >> > Tweet! http://twitter.com/GaryGregory
> > >> >>
> > >> >>
> ---------------------------------------------------------------------
> > >> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> >> For additional commands, e-mail: dev-help@commons.apache.org
> > >> >>
> > >> >>
> > >> >
> > >> >
> > >> > --
> > >> > E-Mail: garydgregory@gmail.com | ggregory@apache.org
> > >> > Java Persistence with Hibernate, Second Edition<
> > >> http://www.manning.com/bauer3/>
> > >> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > >> > Spring Batch in Action <http://www.manning.com/templier/>
> > >> > Blog: http://garygregory.wordpress.com
> > >> > Home: http://garygregory.com/
> > >> > Tweet! http://twitter.com/GaryGregory
> > >>
> > >> ---------------------------------------------------------------------
> > >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> For additional commands, e-mail: dev-help@commons.apache.org
> > >>
> > >>
> > >
> > >
> > > --
> > > E-Mail: garydgregory@gmail.com | ggregory@apache.org
> > > Java Persistence with Hibernate, Second Edition<
> > http://www.manning.com/bauer3/>
> > > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > > Spring Batch in Action <http://www.manning.com/templier/>
> > > Blog: http://garygregory.wordpress.com
> > > Home: http://garygregory.com/
> > > Tweet! http://twitter.com/GaryGregory
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition<
> http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>



-- 
Thanks
Javin
http://javarevisited.blogspot.com/
Twitter : https://twitter.com/javinpaul
Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message