You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by James Leigh <ja...@leighnet.ca> on 2010/04/08 20:18:00 UTC
HttpEntity#writeTo and #consumeContent
Hi all,
After looking through the javadocs and the source code it is not clear
to me when writeTo is called if consumeContent also needs to be called.
The javadoc for HttpEntity#isStreaming says this:
Tells whether this entity depends on an underlying stream.
Streamed entities should return true until the content has been
consumed, false afterwards. Self-contained entities should
return false. Wrapping entities should delegate this call to the
wrapped entity.
The content of a streamed entity is consumed when the stream
returned by getContent has been read to EOF, or after
consumeContent has been called. If a streamed entity can not
detect whether the stream has been read to EOF, it should return
true until consumeContent is called.
This would indicate that consumeContent should be called after writeTo,
unless isStreaming returns false. The implementation for
BasicHttpEntity#writeTo does not close the content stream, which is
consistent with the above (it requires the caller to also call
consumeContent).
// non-javadoc, see interface HttpEntity
public void writeTo(final OutputStream outstream) throws IOException {
if (outstream == null) {
throw new IllegalArgumentException("Output stream may not be null");
}
InputStream instream = getContent();
int l;
byte[] tmp = new byte[2048];
while ((l = instream.read(tmp)) != -1) {
outstream.write(tmp, 0, l);
}
}
However, looking elsewhere in the source code, consumeContent is never
called after writeTo. Here is a code sample from
ThrottlingHttpClientHandler, but similar code can be found in
EntitySerializer.
HttpEntity entity = request.getEntity();
OutputStream outstream = new ContentOutputStream(connState.getOutbuffer());
entity.writeTo(outstream);
outstream.flush();
outstream.close();
When should the content InputStream get closed? Is the bug in
BasicHttpEntity#writeTo or ThrottlingHttpClientHandler and
EntitySerializer?
Thanks for your time,
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#consumeContent deprecated; was Re:
HttpEntity#writeTo and #consumeContent
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Wed, 2010-04-21 at 10:01 -0400, James Leigh wrote:
> On Sat, 2010-04-10 at 17:43 +0200, Oleg Kalnichevski wrote:
> > On Sat, 2010-04-10 at 11:12 -0400, James Leigh wrote:
> > > On Fri, 2010-04-09 at 21:12 +0200, Oleg Kalnichevski wrote:
> > > >
> > > > James et al
> > > >
> > > > I deprecated HttpEntity#consumeContent in favor of a more standard
> > > > InputStream#close contract. Javadocs / tutorial have been updated to
> > > > describe the recommended way to ensure resource deallocation:
> > > >
> > > > http://svn.apache.org/viewvc?rev=932551&view=rev
> > > >
> > > > Please review / comment / critique
> > > >
> > > > Oleg
> > > >
> > >
> > > Thanks for doing this Oleg. However, it is not clear from the revised
> > > javadocs if getContent().close() must be called if the content is not
> > > read.
> >
> > The trouble is this very much depends on the connection management
> > logic, which is out of scope for HttpCore. If one does not intent to
> > re-use the underlying connection is going to close it in any way, there
> > is no point reading the content.
> >
> > I intent to address this issue in the documentation for HttpClient once
> > HttpClient is upgraded to the next version of HttpCore.
>
> > >
> > > While getContent().close() works there are now many ways to clean-up the
> > > same resources and the javadocs need to be very clear. Perhaps more
> > > information in needed in HttpMessage and its sub-interfaces?
> > >
> >
> > I am not much of a writer. I always tend to be too terse with my
> > writing. Please do feel free to improve the javadocs and submit the
> > changes as a patch.
> >
> > Cheers
> >
> > Oleg
> >
>
> How about the following javadoc additions?
>
> For HttpMessage we could say:
>
> Some instances of this require resources to be manually closed. If this
> instance is also an instance of HttpEntityEnclosingRequest or
> HttpResponse and the result of getEntity() is non-null, it must be
> manually closed. See HttpEntity for how to close the resource.
>
> For HttpRequest we could say:
>
> Some instances of this require resources to be manually closed. If this
> instance is also an instance of HttpEntityEnclosingRequest and the
> result of getEntity() is non-null, it must be manually closed. See
> HttpEntity for how to close the resource.
>
> For HttpEntityEnclosingRequest we could say:
>
> Instances of this interface may require resources to be manually closed.
> If the result of getEntity() is non-null, it must be manually closed.
> See HttpEntity for how to close the resource.
>
> For HttpResponse we could say:
>
> Instances of this interface may require resources to be manually closed.
> If the result of getEntity() is non-null, it must be manually closed.
> See HttpEntity for how to close the resource.
>
> For HttpEntity we could say:
>
> Instances of this interface require resources to be manually closed. To
> free up any used resources every instance of HttpEntity must either call
> #writeTo or InputStream#close() on the resust of #getContent() or if
> this is also an instance of ProducingNHttpEntity or ConsumingNHttpEntity
> #finish()
>
> For ProducingNHttpEntity we could say:
>
> See HttpEntity on how to close this resource.
>
> For ConsumingNHttpEntity we could say:
>
> See HttpEntity on how to close this resource.
>
> James
>
James,
It would be much easier for me if could submit these changes as a patch
in udiff format against SVN trunk.
Evil Extortionist Oleg
PS:
I have also been thinking about using generics in HttpEntity and related
interfaces to make it more apparent that some entities are backed by a
dynamic stream of data that needs to be properly closed out. Something
along these lines:
interface HttpEntity<T> {
T getContent();
}
interface HttpResponse<T> {
HttpEntity<T> getContent();
}
HttpResponse<InputStream> response = httpclient.execute(...);
HttpEntity<InputStream> entity = response.getEntity();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#consumeContent deprecated; was Re:
HttpEntity#writeTo and #consumeContent
Posted by James Leigh <ja...@leighnet.ca>.
On Sat, 2010-04-10 at 17:43 +0200, Oleg Kalnichevski wrote:
> On Sat, 2010-04-10 at 11:12 -0400, James Leigh wrote:
> > On Fri, 2010-04-09 at 21:12 +0200, Oleg Kalnichevski wrote:
> > >
> > > James et al
> > >
> > > I deprecated HttpEntity#consumeContent in favor of a more standard
> > > InputStream#close contract. Javadocs / tutorial have been updated to
> > > describe the recommended way to ensure resource deallocation:
> > >
> > > http://svn.apache.org/viewvc?rev=932551&view=rev
> > >
> > > Please review / comment / critique
> > >
> > > Oleg
> > >
> >
> > Thanks for doing this Oleg. However, it is not clear from the revised
> > javadocs if getContent().close() must be called if the content is not
> > read.
>
> The trouble is this very much depends on the connection management
> logic, which is out of scope for HttpCore. If one does not intent to
> re-use the underlying connection is going to close it in any way, there
> is no point reading the content.
>
> I intent to address this issue in the documentation for HttpClient once
> HttpClient is upgraded to the next version of HttpCore.
> >
> > While getContent().close() works there are now many ways to clean-up the
> > same resources and the javadocs need to be very clear. Perhaps more
> > information in needed in HttpMessage and its sub-interfaces?
> >
>
> I am not much of a writer. I always tend to be too terse with my
> writing. Please do feel free to improve the javadocs and submit the
> changes as a patch.
>
> Cheers
>
> Oleg
>
How about the following javadoc additions?
For HttpMessage we could say:
Some instances of this require resources to be manually closed. If this
instance is also an instance of HttpEntityEnclosingRequest or
HttpResponse and the result of getEntity() is non-null, it must be
manually closed. See HttpEntity for how to close the resource.
For HttpRequest we could say:
Some instances of this require resources to be manually closed. If this
instance is also an instance of HttpEntityEnclosingRequest and the
result of getEntity() is non-null, it must be manually closed. See
HttpEntity for how to close the resource.
For HttpEntityEnclosingRequest we could say:
Instances of this interface may require resources to be manually closed.
If the result of getEntity() is non-null, it must be manually closed.
See HttpEntity for how to close the resource.
For HttpResponse we could say:
Instances of this interface may require resources to be manually closed.
If the result of getEntity() is non-null, it must be manually closed.
See HttpEntity for how to close the resource.
For HttpEntity we could say:
Instances of this interface require resources to be manually closed. To
free up any used resources every instance of HttpEntity must either call
#writeTo or InputStream#close() on the resust of #getContent() or if
this is also an instance of ProducingNHttpEntity or ConsumingNHttpEntity
#finish()
For ProducingNHttpEntity we could say:
See HttpEntity on how to close this resource.
For ConsumingNHttpEntity we could say:
See HttpEntity on how to close this resource.
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#consumeContent deprecated; was Re:
HttpEntity#writeTo and #consumeContent
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Mon, 2010-04-12 at 09:04 -0400, James Leigh wrote:
> On Sat, 2010-04-10 at 17:43 +0200, Oleg Kalnichevski wrote:
> > On Sat, 2010-04-10 at 11:12 -0400, James Leigh wrote:
> > >
> > > Thanks for doing this Oleg. However, it is not clear from the revised
> > > javadocs if getContent().close() must be called if the content is not
> > > read.
> >
> > The trouble is this very much depends on the connection management
> > logic, which is out of scope for HttpCore. If one does not intent to
> > re-use the underlying connection is going to close it in any way, there
> > is no point reading the content.
> >
> > I intent to address this issue in the documentation for HttpClient once
> > HttpClient is upgraded to the next version of HttpCore.
> >
> > > There needs to be a clear way to free the resources of the
> > > HttpMessage even if the caller does not care to process the HTTP message
> > > body.
> > >
> > > I also want to point out another use-case that is relevant here. Often
> > > the HttpResponse is dependent on external resources, such as a database
> > > connections or a read locks, and these need to be closed when the
> > > response message body is consumed.
> >
> > I am not entirely sure this is a good idea to make HttpResponse
> > responsible for cleanup of such resources. Unless I am missing
> > something, it just does not sound right to me.
> >
> >
> > > By using InputStream#close() as the
> > > signal to free up resources the implementers are required to wrap the
> > > underlying InputSteam to intercept this call and release locks and/or
> > > free db connections. This prevents some read optimizations to occur.
> > >
> > > In the case of a FileInputStream, for example, the OS can bypass
> > > in-memory buffers using FileChannel#transferTo, but a FileInputStream
> > > wound not be detectable if the InputStream was wrapped. However, by
> > > using the ProducingNHttpEntity interface the finish() method allows
> > > implementers to override the clean-up method without interfering with
> > > read optimizations, such as with NFileEntity.
> > >
> > > While getContent().close() works there are now many ways to clean-up the
> > > same resources and the javadocs need to be very clear. Perhaps more
> > > information in needed in HttpMessage and its sub-interfaces?
> > >
> >
> > I am not much of a writer. I always tend to be too terse with my
> > writing. Please do feel free to improve the javadocs and submit the
> > changes as a patch.
> >
> > Cheers
> >
> > Oleg
> >
> >
>
> If it was up to me, I would make HttpMessage extend java.io.Closable say
> the following in all subclass javadoc headers.
>
> #close() must be called when the message is no longer of used to
> free up any resources used by this message.
>
> Would this be possible in a future release?
>
> James
>
That is possible, but would have to wait until 5.0. However, personally,
I would rather keep all resource management at the HTTP entity level.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#consumeContent deprecated; was Re:
HttpEntity#writeTo and #consumeContent
Posted by James Leigh <ja...@leighnet.ca>.
On Sat, 2010-04-10 at 17:43 +0200, Oleg Kalnichevski wrote:
> On Sat, 2010-04-10 at 11:12 -0400, James Leigh wrote:
> >
> > Thanks for doing this Oleg. However, it is not clear from the revised
> > javadocs if getContent().close() must be called if the content is not
> > read.
>
> The trouble is this very much depends on the connection management
> logic, which is out of scope for HttpCore. If one does not intent to
> re-use the underlying connection is going to close it in any way, there
> is no point reading the content.
>
> I intent to address this issue in the documentation for HttpClient once
> HttpClient is upgraded to the next version of HttpCore.
>
> > There needs to be a clear way to free the resources of the
> > HttpMessage even if the caller does not care to process the HTTP message
> > body.
> >
> > I also want to point out another use-case that is relevant here. Often
> > the HttpResponse is dependent on external resources, such as a database
> > connections or a read locks, and these need to be closed when the
> > response message body is consumed.
>
> I am not entirely sure this is a good idea to make HttpResponse
> responsible for cleanup of such resources. Unless I am missing
> something, it just does not sound right to me.
>
>
> > By using InputStream#close() as the
> > signal to free up resources the implementers are required to wrap the
> > underlying InputSteam to intercept this call and release locks and/or
> > free db connections. This prevents some read optimizations to occur.
> >
> > In the case of a FileInputStream, for example, the OS can bypass
> > in-memory buffers using FileChannel#transferTo, but a FileInputStream
> > wound not be detectable if the InputStream was wrapped. However, by
> > using the ProducingNHttpEntity interface the finish() method allows
> > implementers to override the clean-up method without interfering with
> > read optimizations, such as with NFileEntity.
> >
> > While getContent().close() works there are now many ways to clean-up the
> > same resources and the javadocs need to be very clear. Perhaps more
> > information in needed in HttpMessage and its sub-interfaces?
> >
>
> I am not much of a writer. I always tend to be too terse with my
> writing. Please do feel free to improve the javadocs and submit the
> changes as a patch.
>
> Cheers
>
> Oleg
>
>
If it was up to me, I would make HttpMessage extend java.io.Closable say
the following in all subclass javadoc headers.
#close() must be called when the message is no longer of used to
free up any resources used by this message.
Would this be possible in a future release?
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#consumeContent deprecated; was Re:
HttpEntity#writeTo and #consumeContent
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sat, 2010-04-10 at 11:12 -0400, James Leigh wrote:
> On Fri, 2010-04-09 at 21:12 +0200, Oleg Kalnichevski wrote:
> > ...
> >
> > > >
> > > > > I still don't understand what should be done if an IOException occurs
> > > > > (or RuntimeException) while in the read() method. Must consumeContent be
> > > > > called then? Or, in an exception enough to consider the stream released?
> > > > >
> > > >
> > > > When reading from an input stream, #close method should be called from a
> > > > finally clause to ensure proper resource release.
> > > >
> > >
> > > Sounds like you think/agree that writeTo implementations should close
> > > the inputstream in a finally block (since they read from it). I'll file
> > > a bug report and include a patch next week.
> > >
> > > James
> > >
> >
> > James et al
> >
> > I deprecated HttpEntity#consumeContent in favor of a more standard
> > InputStream#close contract. Javadocs / tutorial have been updated to
> > describe the recommended way to ensure resource deallocation:
> >
> > http://svn.apache.org/viewvc?rev=932551&view=rev
> >
> > Please review / comment / critique
> >
> > Oleg
> >
>
> Thanks for doing this Oleg. However, it is not clear from the revised
> javadocs if getContent().close() must be called if the content is not
> read.
The trouble is this very much depends on the connection management
logic, which is out of scope for HttpCore. If one does not intent to
re-use the underlying connection is going to close it in any way, there
is no point reading the content.
I intent to address this issue in the documentation for HttpClient once
HttpClient is upgraded to the next version of HttpCore.
> There needs to be a clear way to free the resources of the
> HttpMessage even if the caller does not care to process the HTTP message
> body.
>
> I also want to point out another use-case that is relevant here. Often
> the HttpResponse is dependent on external resources, such as a database
> connections or a read locks, and these need to be closed when the
> response message body is consumed.
I am not entirely sure this is a good idea to make HttpResponse
responsible for cleanup of such resources. Unless I am missing
something, it just does not sound right to me.
> By using InputStream#close() as the
> signal to free up resources the implementers are required to wrap the
> underlying InputSteam to intercept this call and release locks and/or
> free db connections. This prevents some read optimizations to occur.
>
> In the case of a FileInputStream, for example, the OS can bypass
> in-memory buffers using FileChannel#transferTo, but a FileInputStream
> wound not be detectable if the InputStream was wrapped. However, by
> using the ProducingNHttpEntity interface the finish() method allows
> implementers to override the clean-up method without interfering with
> read optimizations, such as with NFileEntity.
>
> While getContent().close() works there are now many ways to clean-up the
> same resources and the javadocs need to be very clear. Perhaps more
> information in needed in HttpMessage and its sub-interfaces?
>
I am not much of a writer. I always tend to be too terse with my
writing. Please do feel free to improve the javadocs and submit the
changes as a patch.
Cheers
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#consumeContent deprecated; was Re:
HttpEntity#writeTo and #consumeContent
Posted by James Leigh <ja...@leighnet.ca>.
On Fri, 2010-04-09 at 21:12 +0200, Oleg Kalnichevski wrote:
> ...
>
> > >
> > > > I still don't understand what should be done if an IOException occurs
> > > > (or RuntimeException) while in the read() method. Must consumeContent be
> > > > called then? Or, in an exception enough to consider the stream released?
> > > >
> > >
> > > When reading from an input stream, #close method should be called from a
> > > finally clause to ensure proper resource release.
> > >
> >
> > Sounds like you think/agree that writeTo implementations should close
> > the inputstream in a finally block (since they read from it). I'll file
> > a bug report and include a patch next week.
> >
> > James
> >
>
> James et al
>
> I deprecated HttpEntity#consumeContent in favor of a more standard
> InputStream#close contract. Javadocs / tutorial have been updated to
> describe the recommended way to ensure resource deallocation:
>
> http://svn.apache.org/viewvc?rev=932551&view=rev
>
> Please review / comment / critique
>
> Oleg
>
Thanks for doing this Oleg. However, it is not clear from the revised
javadocs if getContent().close() must be called if the content is not
read. There needs to be a clear way to free the resources of the
HttpMessage even if the caller does not care to process the HTTP message
body.
I also want to point out another use-case that is relevant here. Often
the HttpResponse is dependent on external resources, such as a database
connections or a read locks, and these need to be closed when the
response message body is consumed. By using InputStream#close() as the
signal to free up resources the implementers are required to wrap the
underlying InputSteam to intercept this call and release locks and/or
free db connections. This prevents some read optimizations to occur.
In the case of a FileInputStream, for example, the OS can bypass
in-memory buffers using FileChannel#transferTo, but a FileInputStream
wound not be detectable if the InputStream was wrapped. However, by
using the ProducingNHttpEntity interface the finish() method allows
implementers to override the clean-up method without interfering with
read optimizations, such as with NFileEntity.
While getContent().close() works there are now many ways to clean-up the
same resources and the javadocs need to be very clear. Perhaps more
information in needed in HttpMessage and its sub-interfaces?
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
HttpEntity#consumeContent deprecated; was Re: HttpEntity#writeTo
and #consumeContent
Posted by Oleg Kalnichevski <ol...@apache.org>.
...
> >
> > > I still don't understand what should be done if an IOException occurs
> > > (or RuntimeException) while in the read() method. Must consumeContent be
> > > called then? Or, in an exception enough to consider the stream released?
> > >
> >
> > When reading from an input stream, #close method should be called from a
> > finally clause to ensure proper resource release.
> >
>
> Sounds like you think/agree that writeTo implementations should close
> the inputstream in a finally block (since they read from it). I'll file
> a bug report and include a patch next week.
>
> James
>
James et al
I deprecated HttpEntity#consumeContent in favor of a more standard
InputStream#close contract. Javadocs / tutorial have been updated to
describe the recommended way to ensure resource deallocation:
http://svn.apache.org/viewvc?rev=932551&view=rev
Please review / comment / critique
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#writeTo and #consumeContent
Posted by James Leigh <ja...@leighnet.ca>.
On Thu, 2010-04-08 at 21:45 +0200, Oleg Kalnichevski wrote:
> On Thu, 2010-04-08 at 15:18 -0400, James Leigh wrote:
> > On Thu, 2010-04-08 at 20:55 +0200, Oleg Kalnichevski wrote:
> > > On Thu, 2010-04-08 at 14:18 -0400, James Leigh wrote:
> > > > Hi all,
> > > >
> > > > After looking through the javadocs and the source code it is not clear
> > > > to me when writeTo is called if consumeContent also needs to be called.
> > > >
> > >
> > > The choice of the method name is very unfortunate.
> > > HttpEntity#consumeContent should have rather been called
> > > HttpEntity#dispose or HttpEntity#finish.
> > >
> > > Actually the only requirement for proper release of resources held by an
> > > entity is reading the content stream to the end (until a read operation
> > > returns -1). So, any of these should be enough:
> > >
> > > (1) HttpEntity#consumeContent()
> > > (2) HttpEntity#writeTo()
> > > (3) if (HttpEntity#getContent())
> > > HttpEntity#getContent().close()
> > >
> > > I am really thinking about deprecating HttpEntity#consumeContent
> >
> > > >
> > > > When should the content InputStream get closed? Is the bug in
> > > > BasicHttpEntity#writeTo or ThrottlingHttpClientHandler and
> > > > EntitySerializer?
> > > >
> > >
> > > Ideally InputStream returned by HttpEntity#getContent() should always be
> > > closed. This method will ensure the content is fully consumed and
> > > resources held by the entity are properly released.
> > >
> > > Oleg
> >
> >
> > Thanks for you response Oleg,
> >
> >
> > <rant>
> > As a Java programmer, I'm not comfortable with *not* calling an
> > InputStream's close method. I think they should always be closed, even
> > if read() returns -1, but Sun doesn't make it clear either way.
> > </rant>
> >
>
> I'll happily review and apply a patch if you are willing to contribute
> one ;-)
>
>
> > I still don't understand what should be done if an IOException occurs
> > (or RuntimeException) while in the read() method. Must consumeContent be
> > called then? Or, in an exception enough to consider the stream released?
> >
>
> When reading from an input stream, #close method should be called from a
> finally clause to ensure proper resource release.
>
Sounds like you think/agree that writeTo implementations should close
the inputstream in a finally block (since they read from it). I'll file
a bug report and include a patch next week.
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#writeTo and #consumeContent
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Thu, 2010-04-08 at 15:18 -0400, James Leigh wrote:
> On Thu, 2010-04-08 at 20:55 +0200, Oleg Kalnichevski wrote:
> > On Thu, 2010-04-08 at 14:18 -0400, James Leigh wrote:
> > > Hi all,
> > >
> > > After looking through the javadocs and the source code it is not clear
> > > to me when writeTo is called if consumeContent also needs to be called.
> > >
> >
> > The choice of the method name is very unfortunate.
> > HttpEntity#consumeContent should have rather been called
> > HttpEntity#dispose or HttpEntity#finish.
> >
> > Actually the only requirement for proper release of resources held by an
> > entity is reading the content stream to the end (until a read operation
> > returns -1). So, any of these should be enough:
> >
> > (1) HttpEntity#consumeContent()
> > (2) HttpEntity#writeTo()
> > (3) if (HttpEntity#getContent())
> > HttpEntity#getContent().close()
> >
> > I am really thinking about deprecating HttpEntity#consumeContent
>
> > >
> > > When should the content InputStream get closed? Is the bug in
> > > BasicHttpEntity#writeTo or ThrottlingHttpClientHandler and
> > > EntitySerializer?
> > >
> >
> > Ideally InputStream returned by HttpEntity#getContent() should always be
> > closed. This method will ensure the content is fully consumed and
> > resources held by the entity are properly released.
> >
> > Oleg
>
>
> Thanks for you response Oleg,
>
>
> <rant>
> As a Java programmer, I'm not comfortable with *not* calling an
> InputStream's close method. I think they should always be closed, even
> if read() returns -1, but Sun doesn't make it clear either way.
> </rant>
>
I'll happily review and apply a patch if you are willing to contribute
one ;-)
> I still don't understand what should be done if an IOException occurs
> (or RuntimeException) while in the read() method. Must consumeContent be
> called then? Or, in an exception enough to consider the stream released?
>
When reading from an input stream, #close method should be called from a
finally clause to ensure proper resource release.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#writeTo and #consumeContent
Posted by James Leigh <ja...@leighnet.ca>.
On Thu, 2010-04-08 at 20:55 +0200, Oleg Kalnichevski wrote:
> On Thu, 2010-04-08 at 14:18 -0400, James Leigh wrote:
> > Hi all,
> >
> > After looking through the javadocs and the source code it is not clear
> > to me when writeTo is called if consumeContent also needs to be called.
> >
>
> The choice of the method name is very unfortunate.
> HttpEntity#consumeContent should have rather been called
> HttpEntity#dispose or HttpEntity#finish.
>
> Actually the only requirement for proper release of resources held by an
> entity is reading the content stream to the end (until a read operation
> returns -1). So, any of these should be enough:
>
> (1) HttpEntity#consumeContent()
> (2) HttpEntity#writeTo()
> (3) if (HttpEntity#getContent())
> HttpEntity#getContent().close()
>
> I am really thinking about deprecating HttpEntity#consumeContent
> >
> > When should the content InputStream get closed? Is the bug in
> > BasicHttpEntity#writeTo or ThrottlingHttpClientHandler and
> > EntitySerializer?
> >
>
> Ideally InputStream returned by HttpEntity#getContent() should always be
> closed. This method will ensure the content is fully consumed and
> resources held by the entity are properly released.
>
> Oleg
Thanks for you response Oleg,
<rant>
As a Java programmer, I'm not comfortable with *not* calling an
InputStream's close method. I think they should always be closed, even
if read() returns -1, but Sun doesn't make it clear either way.
</rant>
I still don't understand what should be done if an IOException occurs
(or RuntimeException) while in the read() method. Must consumeContent be
called then? Or, in an exception enough to consider the stream released?
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: HttpEntity#writeTo and #consumeContent
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Thu, 2010-04-08 at 14:18 -0400, James Leigh wrote:
> Hi all,
>
> After looking through the javadocs and the source code it is not clear
> to me when writeTo is called if consumeContent also needs to be called.
>
The choice of the method name is very unfortunate.
HttpEntity#consumeContent should have rather been called
HttpEntity#dispose or HttpEntity#finish.
Actually the only requirement for proper release of resources held by an
entity is reading the content stream to the end (until a read operation
returns -1). So, any of these should be enough:
(1) HttpEntity#consumeContent()
(2) HttpEntity#writeTo()
(3) if (HttpEntity#getContent())
HttpEntity#getContent().close()
I am really thinking about deprecating HttpEntity#consumeContent
> The javadoc for HttpEntity#isStreaming says this:
> Tells whether this entity depends on an underlying stream.
> Streamed entities should return true until the content has been
> consumed, false afterwards. Self-contained entities should
> return false. Wrapping entities should delegate this call to the
> wrapped entity.
>
> The content of a streamed entity is consumed when the stream
> returned by getContent has been read to EOF, or after
> consumeContent has been called. If a streamed entity can not
> detect whether the stream has been read to EOF, it should return
> true until consumeContent is called.
>
> This would indicate that consumeContent should be called after writeTo,
> unless isStreaming returns false. The implementation for
> BasicHttpEntity#writeTo does not close the content stream, which is
> consistent with the above (it requires the caller to also call
> consumeContent).
>
>
> // non-javadoc, see interface HttpEntity
> public void writeTo(final OutputStream outstream) throws IOException {
> if (outstream == null) {
> throw new IllegalArgumentException("Output stream may not be null");
> }
> InputStream instream = getContent();
> int l;
> byte[] tmp = new byte[2048];
> while ((l = instream.read(tmp)) != -1) {
> outstream.write(tmp, 0, l);
> }
> }
>
> However, looking elsewhere in the source code, consumeContent is never
> called after writeTo. Here is a code sample from
> ThrottlingHttpClientHandler, but similar code can be found in
> EntitySerializer.
>
> HttpEntity entity = request.getEntity();
> OutputStream outstream = new ContentOutputStream(connState.getOutbuffer());
> entity.writeTo(outstream);
> outstream.flush();
> outstream.close();
>
> When should the content InputStream get closed? Is the bug in
> BasicHttpEntity#writeTo or ThrottlingHttpClientHandler and
> EntitySerializer?
>
Ideally InputStream returned by HttpEntity#getContent() should always be
closed. This method will ensure the content is fully consumed and
resources held by the entity are properly released.
Oleg
> Thanks for your time,
> James
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org