You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2014/02/06 14:53:03 UTC
[fileupload] file stream leaks?
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();
}
?
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
Re: [fileupload] file stream leaks?
Posted by Javin Paul <sa...@gmail.com>.
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 <ga...@gmail.com> wrote:
> On Thu, Feb 6, 2014 at 11:23 AM, sebb <se...@gmail.com> wrote:
>
> > On 6 February 2014 16:06, Gary Gregory <ga...@gmail.com> wrote:
> > > On Thu, Feb 6, 2014 at 10:18 AM, sebb <se...@gmail.com> wrote:
> > >
> > >> On 6 February 2014 15:04, Gary Gregory <ga...@gmail.com>
> wrote:
> > >> > On Thu, Feb 6, 2014 at 9:04 AM, sebb <se...@gmail.com> wrote:
> > >> >
> > >> >> On 6 February 2014 13:53, Gary Gregory <ga...@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
Re: [fileupload] file stream leaks?
Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Feb 6, 2014 at 11:23 AM, sebb <se...@gmail.com> wrote:
> On 6 February 2014 16:06, Gary Gregory <ga...@gmail.com> wrote:
> > On Thu, Feb 6, 2014 at 10:18 AM, sebb <se...@gmail.com> wrote:
> >
> >> On 6 February 2014 15:04, Gary Gregory <ga...@gmail.com> wrote:
> >> > On Thu, Feb 6, 2014 at 9:04 AM, sebb <se...@gmail.com> wrote:
> >> >
> >> >> On 6 February 2014 13:53, Gary Gregory <ga...@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
Re: [fileupload] file stream leaks?
Posted by sebb <se...@gmail.com>.
On 6 February 2014 16:06, Gary Gregory <ga...@gmail.com> wrote:
> On Thu, Feb 6, 2014 at 10:18 AM, sebb <se...@gmail.com> wrote:
>
>> On 6 February 2014 15:04, Gary Gregory <ga...@gmail.com> wrote:
>> > On Thu, Feb 6, 2014 at 9:04 AM, sebb <se...@gmail.com> wrote:
>> >
>> >> On 6 February 2014 13:53, Gary Gregory <ga...@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.
> 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
Re: [fileupload] file stream leaks?
Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Feb 6, 2014 at 10:18 AM, sebb <se...@gmail.com> wrote:
> On 6 February 2014 15:04, Gary Gregory <ga...@gmail.com> wrote:
> > On Thu, Feb 6, 2014 at 9:04 AM, sebb <se...@gmail.com> wrote:
> >
> >> On 6 February 2014 13:53, Gary Gregory <ga...@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?
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
Re: [fileupload] file stream leaks?
Posted by sebb <se...@gmail.com>.
On 6 February 2014 15:04, Gary Gregory <ga...@gmail.com> wrote:
> On Thu, Feb 6, 2014 at 9:04 AM, sebb <se...@gmail.com> wrote:
>
>> On 6 February 2014 13:53, Gary Gregory <ga...@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?
> 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
Re: [fileupload] file stream leaks?
Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Feb 6, 2014 at 9:04 AM, sebb <se...@gmail.com> wrote:
> On 6 February 2014 13:53, Gary Gregory <ga...@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?
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
Re: [fileupload] file stream leaks?
Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Feb 6, 2014 at 9:04 AM, sebb <se...@gmail.com> wrote:
> On 6 February 2014 13:53, Gary Gregory <ga...@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?
>
I can imagine some confusion about having both the util Streams class AND
IOUtils around.
I would change Stream impl to call IOUtils. Since we already depend on
[io], we might as well use it.
Gary
> The close should perhaps be
>
> IOUtils.closeQuietly()
>
> Are we interested in knowibg about input close() failures?
>
> > 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
Re: [fileupload] file stream leaks?
Posted by sebb <se...@gmail.com>.
On 6 February 2014 13:53, Gary Gregory <ga...@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?
> 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