atom feed24 messages in org.apache.ws.axis-devRe: HTTP connection leak and other re...
FromSent OnAttachments
Alexis MidonFeb 26, 2009 5:54 pm 
Dobri KitipovFeb 27, 2009 1:10 am 
Andreas VeithenFeb 27, 2009 2:45 am 
Dobri KitipovFeb 27, 2009 3:19 am 
Andreas VeithenFeb 27, 2009 5:04 am 
Andreas VeithenFeb 27, 2009 2:59 pm 
Alexis MidonFeb 27, 2009 5:52 pm 
Amila SuriarachchiMar 1, 2009 9:02 pm 
Alexis MidonMar 2, 2009 10:10 am 
Alexis MidonMar 3, 2009 4:21 pm 
Andreas VeithenMar 9, 2009 1:56 pm 
Amila SuriarachchiMar 22, 2009 3:52 am 
Dobri KitipovMar 23, 2009 2:20 am 
Amila SuriarachchiMar 23, 2009 4:08 am 
Dobri KitipovMar 23, 2009 5:29 am 
Alexis MidonMar 23, 2009 3:45 pm 
Alexis MidonMar 23, 2009 4:44 pm 
Amila SuriarachchiMar 23, 2009 9:20 pm 
Alexis MidonMar 24, 2009 9:40 am 
Dobri KitipovMar 25, 2009 3:10 am 
Alexis MidonMar 25, 2009 9:16 am 
Alexis MidonMar 25, 2009 9:45 am 
Dobri KitipovMar 26, 2009 2:41 am 
Alexis MidonMar 26, 2009 3:46 pm 
Subject:Re: HTTP connection leak and other related issues
From:Dobri Kitipov (kdob@googlemail.com)
Date:Mar 26, 2009 2:41:32 am
List:org.apache.ws.axis-dev

Hi Alexis,

I suppose there is a miscommunication here. We have so many use-cases in our heads. And this thread is becoming so long :).

I suppose you are talking about the use case when the "HTTPConstants.REUSE_HTTP_CLIENT" property is *NOT* set to "true", so there is no HttpClient reuse from one client's invocation to another? If this is the case, I can fully agree with you.

The lines of code we are talking about are part of AbstractHTTPSender#getHttpClient. Here is the code excerpt (the else part of the if) when "HTTPConstants.REUSE_HTTP_CLIENT" property is *NOT* set to "true":

else { HttpConnectionManager connManager = (HttpConnectionManager) msgContext.getProperty(

HTTPConstants.MULTITHREAD_HTTP_CONNECTION_MANAGER); if (connManager == null) { connManager = (HttpConnectionManager) msgContext.getProperty(

HTTPConstants.MUTTITHREAD_HTTP_CONNECTION_MANAGER); } if(connManager != null){ httpClient = new HttpClient(connManager); } else { //Multi threaded http connection manager has set as the default --------->> connManager = new MultiThreadedHttpConnectionManager(); httpClient = new HttpClient(connManager); } }

Thanks, Dobri

On Wed, Mar 25, 2009 at 6:16 PM, Alexis Midon <mid@intalio.com> wrote:

No sure I'm getting what you're saying. By default Axis2 creates a new HttpClient instance and a new HttpConnectionManager instance on every invocations. So multiple invocations should have step on each other in the default case. That's why I said MultiThreadedHttpConnectionManager seems overkill to me.

Alexis

On Wed, Mar 25, 2009 at 3:11 AM, Dobri Kitipov < kdob@googlemail.com> wrote:

Hi everybody, I am not sure that the default ConnectionManager should be changed from MultiThreadedHttpConnectionManager to SimpleHttpConnectionManager. What about asynchronous MEPs? I have not tested this, but I suppose that if a given async client does several invocations then if *NO* MultiThreadedHttpConnectionManager is in use (i.e. there is no connection pool) then the NON-blocking mechanism will be corrupted?

Dobri

On Tue, Mar 24, 2009 at 6:40 PM, Alexis Midon <mid@intalio.com> wrote:

+1 on this. I'm fine with the current behavior regarding HttpClient management. My only request would be to not instanciate a MultiThreadedHttpConnectionManager in the default case. A SimpleHttpConnectionManager would be good enough. I agree it's close to be a detail but still.

Alexis

On Mon, Mar 23, 2009 at 9:20 PM, Amila Suriarachchi < amil@gmail.com> wrote:

On Tue, Mar 24, 2009 at 4:16 AM, Alexis Midon <mid@intalio.com>wrote:

It's already possible to reuse the HttpClient and with your own HttpConnectionManager. What you have to do is create the HttpClient by yourself with your connection manager and push it in the MessageContext.

I think this is a good point. So if we set both HTTPConstants.REUSE_HTTP_CLIENT, HTTPConstants.CACHED_HTTP_CLIENT then ultimately transport sender use that client. This means users can manage the way they need to use the httpClient at application level. either they can use one httpClient for all the messages or kept the HttpState at thread local. So IMHO since this is a performance fine tuning, it is ok to keep it in this way and let users to mange it in the way they need at application level.

thanks, Amila.

Alexis

On Sun, Mar 22, 2009 at 3:52 AM, Amila Suriarachchi < amil@gmail.com> wrote:

hi all,

recently I came across this link and according to this[1] creating new http client for every request is not good in performance wise.

Current get Http client method looks like this

protected HttpClient getHttpClient(MessageContext msgContext) { HttpClient httpClient; Object reuse = msgContext.getOptions().getProperty(HTTPConstants.REUSE_HTTP_CLIENT); if (reuse == null) { reuse = msgContext.getConfigurationContext().getProperty(HTTPConstants.REUSE_HTTP_CLIENT); } if (reuse != null && JavaUtils.isTrueExplicitly(reuse)) { httpClient = (HttpClient) msgContext.getOptions().getProperty(HTTPConstants.CACHED_HTTP_CLIENT); if (httpClient == null) { httpClient = (HttpClient) msgContext.getConfigurationContext()

.getProperty(HTTPConstants.CACHED_HTTP_CLIENT); } if (httpClient != null) return httpClient; MultiThreadedHttpConnectionManager connectionManager = new MultiThreadedHttpConnectionManager(); httpClient = new HttpClient(connectionManager); msgContext.getConfigurationContext() .setProperty(HTTPConstants.CACHED_HTTP_CLIENT, httpClient); } else { HttpConnectionManager connManager = (HttpConnectionManager) msgContext.getProperty(

HTTPConstants.MULTITHREAD_HTTP_CONNECTION_MANAGER); if (connManager == null) { connManager = (HttpConnectionManager) msgContext.getProperty(

HTTPConstants.MUTTITHREAD_HTTP_CONNECTION_MANAGER); } if(connManager != null){ httpClient = new HttpClient(connManager); } else { //Multi threaded http connection manager has set as the default connManager = new MultiThreadedHttpConnectionManager(); httpClient = new HttpClient(connManager); } }

// Get the timeout values set in the runtime initializeTimeouts(msgContext, httpClient); return httpClient; }

According to the above article creating many http client instances are not efficient. But if a user switch on the HTTPConstants.REUSE_HTTP_CLIENT then he can not give an MultiThreadedHttpConnectionManager object where users can define maximum pool size, host configurations etc...

Therefore I think it is better to have these features,

1. Let users to create the MultiThreadedHttpConnectionManager with its own even when reusing http client. 2. Does Http Client thread safe? if not we need to keep the cache copy of the httpClient in a thread local variable.

Does any one knows how to switch on the keep alive and will it make any performance improvement?

Anyway I still believe releasing connections should be done at the user level since it degradates the performance.

thanks, Amila.

[1] http://hc.apache.org/httpclient-3.x/performance.html

On Tue, Mar 10, 2009 at 2:26 AM, Andreas Veithen < andr@gmail.com> wrote:

I agree that we also need to consider OperationClient and the non-blocking methods. I think that the first step towards a solid solution is actually to write a couple of test cases that provide evidence for these issues and that we can later use for regression testing, but I will have to review the existing unit tests we have.

Andreas

On Wed, Mar 4, 2009 at 01:21, Alexis Midon <mid@intalio.com> wrote:

Another case where "Options#setCallTransportCleanup for OperationClient" is obvious is when you call OperationClient#execute in a non-blocking way. The caller cannot clean up the transport safely, because the execution might still be in progress. In that case it's OperationClient responsability to clean up the transport.

Alexis

On Mon, Mar 2, 2009 at 10:11 AM, Alexis Midon <mid@intalio.com>

wrote:

There is still some inconsistencies between how ServiceClient#sendReceive and Operation#execute use Options#getCallTransportCleanup.

And I think that would help a lot if the related jira issues get cleaned up a little bit.

Thanks for your time and feedback.

Alexis

On Sun, Mar 1, 2009 at 9:02 PM, Amila Suriarachchi <amil@gmail.com> wrote:

hi all,

On Fri, Feb 27, 2009 at 6:35 PM, Andreas Veithen <andr@gmail.com> wrote:

I think that callTransportCleanup should never be turned on by default because it would disable deferred parsing of the response. What needs to be done urgently is to improve the documentation of the ServiceClient class to make it clear that it is mandatory to either call cleanupTransport explicitly or to set callTransportCleanup to true. Also the cleanupTransport method itself doesn't have any Javadoc.

thanks for in-depth analysing of this issue. If the issue is not calling to transport clean up then I clearly agree with what Andreas.

Axis2 uses deffered building. There when the user gets the response OMElement it has not build and Axis2 does not know when he going to finish with it. So it is responsibility of the user to call the transport clean up once they have done with the OMElement.

But this problem does not occur with the generated data bind code. There before returning from the stub method it generates the all the databinding classes. In the generated code finally method calls

_messageContext.getTransportOut().getSender().cleanup(_messageContext);

to clean up the transport.

thanks, Amila.

Andreas

On Fri, Feb 27, 2009 at 12:19, Dobri Kitipov <kdob@googlemail.com> wrote:

Hi Andreas, thank you for the comment. I think you get the question. Quick test shows that setting the following line of code into the client:

options.setCallTransportCleanup(true);

forces the closure of the http connection. It seems it is not the default behavior. This is a good and fast solution. I was a little bit more focused on wondering why I have such a difference b/n using SOAP and MIME builder.

I need to think about some use cases when we need to have options.setCallTransportCleanup(false). Can we have this by default in some cases?

Anyway, it will be worth having a further analysis of the issue we have with SOAPBuilder behavior.

Thank you, Dobri

On Fri, Feb 27, 2009 at 12:46 PM, Andreas Veithen <andr@gmail.com> wrote:

If I understand correctly, Dobri's findings can be summarized as follows: 1. Once the InputStream is consumed, commons-httpclient automatically releases the connection. 2. SOAPBuilder never completely consumes the InputStream.

The SOAPBuilder behavior is indeed somewhat questionable, but it is important to understand that because of the deferred parsing model used by Axiom, there is never a guarantee that the InputStream will be completely consumed. Normally releasing the connection is the responsibility of the CommonsHTTPTransportSender#cleanup method which should be called by ServiceClient#cleanupTransport. It would be interesting to know if that method is called, and if it is, why it fails to release the connection.

Andreas

On Fri, Feb 27, 2009 at 10:10, Dobri Kitipov <kdob@googlemail.com> wrote:

Hi all, I have observed absolutely the same thing these days. I need some more time to analyze the whole picture, but here is my current synthesis of the issue.

It seems that http connection release is tightly coupled with the Axis2 builder used to handle and process the response body and the corresponding input streams used. The builder and the InputStream used are based on the HTTP headers fields like "Content-Type" and "Transfer-Encoding" (e.g. Content-Type: text/xml; charset=UTF-8 Transfer-Encoding: chunked). So if we have Content-Type: text/xml; then SOAPBuilder class will be used. If we have type="application/xop+xml" then MIMEBuilder will be used.

The successfull story when we have MIMIBuilder:

When MIMEBuilder is used then the response Buffered InputStream (IS) is wrrapped (I will use "->" sign as substitution for wrrapped) ChunkedIS -> AutoCloseIS (this has a responseConsumed() method notified when IS.read() returns -1, which means that the response IS has been completely read) -> PushBackIS ->BounderyPushBackIS. The BounderyPushBackIS reads the response stream (see readFromStream(....)) in a cycle till it reaches its end. At every iteration of this cycle a AutoCloseIS checkClose(l) is invoked. So when the end is reached (-1 is returned) then this check causes the invokation of the AutoCloseIS checkClose(...) method. This method invokes notifyWatcher() that in turn invokes responseBodyConsumed() method of the HttpMethodBase class. This causes the release of the http connection which is returned back to the connection pool. So here we have no problem with connection reuse.

The bad story we have with SOAPBuilder:

When SOAPBuilder is used then the response Buffered InputStream is wrrapped in a ChunkedIS -> AutoCloseIS -> PushBackIS. May be you has noticed that BounderyPushBackIS is not used. As a result the respose IS is not completely read (in fact this is not really correct, it could be read but the invokation of the PushBackIS unread(...) method causes the AutoCloseIS checkClose() method never to return -1). As a result the http connection is not released. And since there is a limit to have 2 connection per host then after the second invokation of the WS client the thread hangs waiting for a free connection.

Please, provide us with your comments on this issue.

Thank you in advance.

Regards, Dobri

On Fri, Feb 27, 2009 at 3:54 AM, Alexis Midon < mid@intalio.com> wrote:

no taker for an easy patch?

Alexis

On Wed, Feb 25, 2009 at 6:45 PM, Alexis Midon < mid@intalio.com> wrote:

Hi everyone,

All the issues relatives to AXIS2-935 are really messy, some of them are closed but their clones are not. Some are flagged as fixed but are obviously not. All these issues are really old, so I'd like to take a chance to bring them back to your attention, especially before releasing 1.5.

I'll post a description of the issue in this email as a summary all the jiras.

By default, ServiceClient uses one HttpConnectionManager per invocation [2]. This connection manager will create and provide one connection to HTTPSender. The first issue is that by default this connection is never released to the pool [3]. if you do zillions of invocations, this leak will max out your number of file descriptors.

Your investigations in Axis2 options quickly lead you to the REUSE_HTTP_CLIENT option. But this first issue has some unfortunate consequences if you activate it. Actually if you do so, a single connection manager is shared across all invocations. But because connections are not release, the pool is starved after two invocations, and the third invocation hangs out indefinitely. :(

If you keep digging you will find the AUTO_RELEASE_CONNECTION option. Its sounds like a good lead! Let's try it. If you activate this option the connection is properly released -Yahoooo! the leak is fixed - but unfortunately a new issue shows up (issue #2, aka AXIS2-3478). AbstractHTTPSender passes the stream of the connection to the message context [4] , but that the connection is now properly released, so this stream is closed before the SOAPBuilder gets a chance to read the response body. Boom! "IOException: Attempted read on closed stream"

These issues are easily reproducible in versions 1.3, 1.4, 1.5.

I submitted and documented a fix in AXIS2-2931 [5], if you had a chance to look at it that would be much appreciate.

Alexis

[1]

https://issues.apache.org/jira/browse/AXIS2-935?focusedCommentId=12513543#action_12513543

[2] see method getHttpClient in

https://svn.apache.org/repos/asf/webservices/commons/trunk/modules/transport/modules/http/src/org/apache/axis2/transport/http/AbstractHTTPSender.java

[3] see method cleanup in

https://svn.apache.org/repos/asf/webservices/commons/trunk/modules/transport/modules/http/src/org/apache/axis2/transport/http/HTTPSender.java

[4] see method processResponse in AbstractHTTPSender.java [5]

https://issues.apache.org/jira/browse/AXIS2-2931?focusedCommentId=12676837#action_12676837

-- Amila Suriarachchi WSO2 Inc. blog: http://amilachinthaka.blogspot.com/

-- Amila Suriarachchi WSO2 Inc. blog: http://amilachinthaka.blogspot.com/

-- Amila Suriarachchi WSO2 Inc. blog: http://amilachinthaka.blogspot.com/