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 <mi...@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 <mi...@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 >> <amilasuriarach...@gmail.com> wrote: >>> >>> hi all, >>> >>> On Fri, Feb 27, 2009 at 6:35 PM, Andreas Veithen >>> <andreas.veit...@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 >>>> <kdobrik.ax...@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 >>>> > <andreas.veit...@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 >>>> >> <kdobrik.ax...@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 <mi...@intalio.com> >>>> >> > wrote: >>>> >> >> >>>> >> >> no taker for an easy patch? >>>> >> >> >>>> >> >> Alexis >>>> >> >> >>>> >> >> >>>> >> >> On Wed, Feb 25, 2009 at 6:45 PM, Alexis Midon <mi...@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/ >> > >