You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by danch <da...@danch.com> on 2003/09/24 20:12:39 UTC
connection leak in HttpMethodBase when HttpRecoverableException is
re-thrown
Hello,
I'm using HttpClient (2.0 rc1) under Axis (using their CommonsHttpSender class) and we've run into
some problems with a web application locking up (deadlock).
A little research and a test application revealed that this our application arrived at this
condition after receiving a couple of exceptions from HttpMethodBase.processRequest. If
processRequest catches an HttpRecoverableException, it attempts to determine whether a retry should
be attempted or not, and if not (which is our case), it rethrows the exception, which is propogated
on up the stack:
} catch (HttpRecoverableException httpre) {
if (LOG.isDebugEnabled()) {
LOG.debug("Closing the connection.");
}
connection.close();
LOG.info("Recoverable exception caught when processing request");
// update the recoverable exception count.
recoverableExceptionCount++;
// test if this method should be retried
if (!getMethodRetryHandler().retryMethod(
this,
connection,
httpre,
execCount,
requestSent)
) {
LOG.warn("Recoverable exception caught but MethodRetryHandler.retryMethod() "
+ "returned false, rethrowing exception"
);
throw httpre;
}
}
The caller of processRequest (HttpMethodBase.execute) does not handle the exception, but has the
following finally block:
} finally {
inExecute = false;
// If the response has been fully processed, return the connection
// to the pool. Use this flag, rather than other tests (like
// responseStream == null), as subclasses, might reset the stream,
// for example, reading the entire response into a file and then
// setting the file as the stream.
if (doneWithConnection) {
ensureConnectionRelease();
}
}
Note that the retry case is handled internally to HttpMethodBase.execute. I believe that the
'doneWithConnection flag should be set to true before rethrowing the HttpRecoverableException in the
above catch block, so that the connection will be released properly. This change seems to have
solved the problem we were having. Here's diff -u output
--- HttpMethodBase.java 2003-09-24 13:10:10.249937500 -0500
+++ HttpMethodBase.mine 2003-09-23 16:51:31.394929000 -0500
@@ -2644,6 +2644,7 @@
"Recoverable exception caught but MethodRetryHandler.retryMethod() "
+ "returned false, rethrowing exception"
);
+ doneWithConnection = true;
throw httpre;
}
}
thanks,
danch
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
Re: connection leak in HttpMethodBase when HttpRecoverableException
is re-thrown
Posted by danch <da...@danch.com>.
Thanks Michael. If you determine that HttpClient shouldn't be automatically releasing in this case,
I'll take the issue up on the Axis side.
thanks again,
danch
Michael Becke wrote:
>
> I will investigate automatic release for this case and will let you know
> what I come up with. If anyone else has some ideas they would be
> appreciated.
>
> Thanks,
>
> Mike
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
Re: connection leak in HttpMethodBase when HttpRecoverableException is re-thrown
Posted by Michael Becke <be...@u.washington.edu>.
danch,
I agree, it seems like HttpMethodBase should be releasing the
connection automatically when an exception occurs in processRequest.
Either there is some case when a valid response can still be read when
this happens, or this is just an oversight. In either case, HttpClient
requires that releaseConnection() be called for all method executions.
This goes for successful as well as unsuccessful calls. HttpClient
does it's best to ensure that connections are automatically released
when possible, but it is ultimately up to the user.
I will investigate automatic release for this case and will let you
know what I come up with. If anyone else has some ideas they would be
appreciated.
Thanks,
Mike
On Wednesday, September 24, 2003, at 03:48 PM, danch wrote:
> Right, but my question is where should it be called? Or really, why is
> the HttpClient code calling it in some exception cases but not others?
>
>
> Michael Becke wrote:
>> danch,
>> Method.releaseConnection() should be called for all methods executed,
>> including those that end in an exception. This is the general rule,
>> though it should only be necessary for methods that could potentially
>> return a response. If the response will be read outside of the
>> context of HttpClient.execute() then closing or fully reading the
>> response stream is sufficient. In short, always calling
>> releaseConnection() is the way to go.
>> Mike
>> danch wrote:
>>> from where? Should HttpClient's client (Axis in this case) be calling
>>> releaseConnection in the exception case? Normally, Axis is relying on
>>> AutoCloseInputStream to route the release back into HttpClient via
>>> ResponseConsumeWatcher. I was unsure on where this should
>>> be and decided on this fix because some exception cases _are_
>>> handled internally to
>>> HttpClient (IOException from connection.open is handled in
>>> HttpClient.executeMethod, for example).
>>>
>>> What is the rule for who should call the releaseConnection method
>>> when?
>>>
>>> thanks,
>>> danch
>>>
>>> Michael Becke wrote:
>>>
>>>> Danch,
>>>>
>>>> Are you calling method.releaseConnection() in the exception case?
>>>>
>>>> Mike
>>>>
>>>> danch wrote:
>>>>
>>>>> Thanks for the reply.
>>>>> I pulled down the release2 branch to do this testing. The branch
>>>>> built without my patch locks up in 'doGetConnection' if I start a
>>>>> multithreaded test then kill the webservices server, causing
>>>>> IOExceptions as I described.
>>>>>
>>>>> I'm rather leary of trying HEAD both because I need stable code,
>>>>> and because the Axis CommonsHttpSender is written to the 2.0 apis
>>>>> - I'm not sure if anything significant has changed or not. I can
>>>>> give this a shot and see if the current HEAD exhibits the same
>>>>> behavior, but I probably won't be able to do that until tomorrow.
>>>>>
>>>>> I'd seen bug 23137 - the patch for that seems to be for HEAD, not
>>>>> the release 2 branch.
>>>>>
>>>>> Actually, I'd also seen bug 22841 - that fixes releases where an
>>>>> exception pops out of the close call in releaseConnection. In my
>>>>> case, the problem was that releaseConnection was never called.
>>>>>
>>>>> thanks again,
>>>>> danch
>>>>>
>>>>> Eric Johnson wrote:
>>>>>
>>>>>> Danch,
>>>>>>
>>>>>> Note the bugs:
>>>>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=23137
>>>>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=22841
>>>>>>
>>>>>> If you can, pull the latest code from CVS, or grab a nightly
>>>>>> build, and try it again, as the bug may very well be fixed
>>>>>> already.
>>>>>>
>>>>>> Let us know how it goes.
>>>>>>
>>>>>> - Eric
>>>>>>
>>>>>> danch wrote:
>>>>>>
>>>>>>> Hello,
>>>>>>> I'm using HttpClient (2.0 rc1) under Axis (using their
>>>>>>> CommonsHttpSender class) and we've run into some problems with a
>>>>>>> web application locking up (deadlock).
>>>>>
>>>>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail:
> commons-httpclient-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
Re: connection leak in HttpMethodBase when HttpRecoverableException
is re-thrown
Posted by danch <da...@danch.com>.
Right, but my question is where should it be called? Or really, why is the HttpClient code calling
it in some exception cases but not others?
Michael Becke wrote:
> danch,
>
> Method.releaseConnection() should be called for all methods executed,
> including those that end in an exception. This is the general rule,
> though it should only be necessary for methods that could potentially
> return a response. If the response will be read outside of the context
> of HttpClient.execute() then closing or fully reading the response
> stream is sufficient. In short, always calling releaseConnection() is
> the way to go.
>
> Mike
>
> danch wrote:
>
>> from where? Should HttpClient's client (Axis in this case) be calling
>> releaseConnection in the exception case? Normally, Axis is relying on
>> AutoCloseInputStream to route the release back into HttpClient via
>> ResponseConsumeWatcher. I was unsure on where this should
>> be and decided on this fix because some exception cases _are_ handled
>> internally to
>> HttpClient (IOException from connection.open is handled in
>> HttpClient.executeMethod, for example).
>>
>> What is the rule for who should call the releaseConnection method when?
>>
>> thanks,
>> danch
>>
>> Michael Becke wrote:
>>
>>> Danch,
>>>
>>> Are you calling method.releaseConnection() in the exception case?
>>>
>>> Mike
>>>
>>> danch wrote:
>>>
>>>> Thanks for the reply.
>>>> I pulled down the release2 branch to do this testing. The branch
>>>> built without my patch locks up in 'doGetConnection' if I start a
>>>> multithreaded test then kill the webservices server, causing
>>>> IOExceptions as I described.
>>>>
>>>> I'm rather leary of trying HEAD both because I need stable code, and
>>>> because the Axis CommonsHttpSender is written to the 2.0 apis - I'm
>>>> not sure if anything significant has changed or not. I can give this
>>>> a shot and see if the current HEAD exhibits the same behavior, but I
>>>> probably won't be able to do that until tomorrow.
>>>>
>>>> I'd seen bug 23137 - the patch for that seems to be for HEAD, not
>>>> the release 2 branch.
>>>>
>>>> Actually, I'd also seen bug 22841 - that fixes releases where an
>>>> exception pops out of the close call in releaseConnection. In my
>>>> case, the problem was that releaseConnection was never called.
>>>>
>>>> thanks again,
>>>> danch
>>>>
>>>> Eric Johnson wrote:
>>>>
>>>>> Danch,
>>>>>
>>>>> Note the bugs:
>>>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=23137
>>>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=22841
>>>>>
>>>>> If you can, pull the latest code from CVS, or grab a nightly build,
>>>>> and try it again, as the bug may very well be fixed already.
>>>>>
>>>>> Let us know how it goes.
>>>>>
>>>>> - Eric
>>>>>
>>>>> danch wrote:
>>>>>
>>>>>> Hello,
>>>>>> I'm using HttpClient (2.0 rc1) under Axis (using their
>>>>>> CommonsHttpSender class) and we've run into some problems with a
>>>>>> web application locking up (deadlock).
>>>>
>>>>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
Re: connection leak in HttpMethodBase when HttpRecoverableException
is re-thrown
Posted by Michael Becke <be...@u.washington.edu>.
danch,
Method.releaseConnection() should be called for all methods executed,
including those that end in an exception. This is the general rule,
though it should only be necessary for methods that could potentially
return a response. If the response will be read outside of the context
of HttpClient.execute() then closing or fully reading the response
stream is sufficient. In short, always calling releaseConnection() is
the way to go.
Mike
danch wrote:
> from where? Should HttpClient's client (Axis in this case) be calling
> releaseConnection in the exception case? Normally, Axis is relying on
> AutoCloseInputStream to route the release back into HttpClient via
> ResponseConsumeWatcher. I was unsure on where this should
> be and decided on this fix because some exception cases _are_ handled
> internally to
> HttpClient (IOException from connection.open is handled in
> HttpClient.executeMethod, for example).
>
> What is the rule for who should call the releaseConnection method when?
>
> thanks,
> danch
>
> Michael Becke wrote:
>
>> Danch,
>>
>> Are you calling method.releaseConnection() in the exception case?
>>
>> Mike
>>
>> danch wrote:
>>
>>> Thanks for the reply.
>>> I pulled down the release2 branch to do this testing. The branch
>>> built without my patch locks up in 'doGetConnection' if I start a
>>> multithreaded test then kill the webservices server, causing
>>> IOExceptions as I described.
>>>
>>> I'm rather leary of trying HEAD both because I need stable code, and
>>> because the Axis CommonsHttpSender is written to the 2.0 apis - I'm
>>> not sure if anything significant has changed or not. I can give this
>>> a shot and see if the current HEAD exhibits the same behavior, but I
>>> probably won't be able to do that until tomorrow.
>>>
>>> I'd seen bug 23137 - the patch for that seems to be for HEAD, not the
>>> release 2 branch.
>>>
>>> Actually, I'd also seen bug 22841 - that fixes releases where an
>>> exception pops out of the close call in releaseConnection. In my
>>> case, the problem was that releaseConnection was never called.
>>>
>>> thanks again,
>>> danch
>>>
>>> Eric Johnson wrote:
>>>
>>>> Danch,
>>>>
>>>> Note the bugs:
>>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=23137
>>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=22841
>>>>
>>>> If you can, pull the latest code from CVS, or grab a nightly build,
>>>> and try it again, as the bug may very well be fixed already.
>>>>
>>>> Let us know how it goes.
>>>>
>>>> - Eric
>>>>
>>>> danch wrote:
>>>>
>>>>> Hello,
>>>>> I'm using HttpClient (2.0 rc1) under Axis (using their
>>>>> CommonsHttpSender class) and we've run into some problems with a
>>>>> web application locking up (deadlock).
>>>
>>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail:
> commons-httpclient-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
Re: connection leak in HttpMethodBase when HttpRecoverableException
is re-thrown
Posted by danch <da...@danch.com>.
from where? Should HttpClient's client (Axis in this case) be calling
releaseConnection in the exception case? Normally, Axis is relying on
AutoCloseInputStream to route the release back into HttpClient via
ResponseConsumeWatcher. I was unsure on where this should
be and decided on this fix because some exception cases _are_ handled internally to
HttpClient (IOException from connection.open is handled in
HttpClient.executeMethod, for example).
What is the rule for who should call the releaseConnection method when?
thanks,
danch
Michael Becke wrote:
> Danch,
>
> Are you calling method.releaseConnection() in the exception case?
>
> Mike
>
> danch wrote:
>
>> Thanks for the reply.
>> I pulled down the release2 branch to do this testing. The branch built
>> without my patch locks up in 'doGetConnection' if I start a
>> multithreaded test then kill the webservices server, causing
>> IOExceptions as I described.
>>
>> I'm rather leary of trying HEAD both because I need stable code, and
>> because the Axis CommonsHttpSender is written to the 2.0 apis - I'm
>> not sure if anything significant has changed or not. I can give this a
>> shot and see if the current HEAD exhibits the same behavior, but I
>> probably won't be able to do that until tomorrow.
>>
>> I'd seen bug 23137 - the patch for that seems to be for HEAD, not the
>> release 2 branch.
>>
>> Actually, I'd also seen bug 22841 - that fixes releases where an
>> exception pops out of the close call in releaseConnection. In my case,
>> the problem was that releaseConnection was never called.
>>
>> thanks again,
>> danch
>>
>> Eric Johnson wrote:
>>
>>> Danch,
>>>
>>> Note the bugs:
>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=23137
>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=22841
>>>
>>> If you can, pull the latest code from CVS, or grab a nightly build,
>>> and try it again, as the bug may very well be fixed already.
>>>
>>> Let us know how it goes.
>>>
>>> - Eric
>>>
>>> danch wrote:
>>>
>>>> Hello,
>>>> I'm using HttpClient (2.0 rc1) under Axis (using their
>>>> CommonsHttpSender class) and we've run into some problems with a web
>>>> application locking up (deadlock).
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
Re: connection leak in HttpMethodBase when HttpRecoverableException
is re-thrown
Posted by Michael Becke <be...@u.washington.edu>.
Danch,
Are you calling method.releaseConnection() in the exception case?
Mike
danch wrote:
> Thanks for the reply.
> I pulled down the release2 branch to do this testing. The branch built
> without my patch locks up in 'doGetConnection' if I start a
> multithreaded test then kill the webservices server, causing
> IOExceptions as I described.
>
> I'm rather leary of trying HEAD both because I need stable code, and
> because the Axis CommonsHttpSender is written to the 2.0 apis - I'm not
> sure if anything significant has changed or not. I can give this a shot
> and see if the current HEAD exhibits the same behavior, but I probably
> won't be able to do that until tomorrow.
>
> I'd seen bug 23137 - the patch for that seems to be for HEAD, not the
> release 2 branch.
>
> Actually, I'd also seen bug 22841 - that fixes releases where an
> exception pops out of the close call in releaseConnection. In my case,
> the problem was that releaseConnection was never called.
>
> thanks again,
> danch
>
> Eric Johnson wrote:
>
>> Danch,
>>
>> Note the bugs:
>> http://issues.apache.org/bugzilla/show_bug.cgi?id=23137
>> http://issues.apache.org/bugzilla/show_bug.cgi?id=22841
>>
>> If you can, pull the latest code from CVS, or grab a nightly build,
>> and try it again, as the bug may very well be fixed already.
>>
>> Let us know how it goes.
>>
>> - Eric
>>
>> danch wrote:
>>
>>> Hello,
>>> I'm using HttpClient (2.0 rc1) under Axis (using their
>>> CommonsHttpSender class) and we've run into some problems with a web
>>> application locking up (deadlock).
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail:
> commons-httpclient-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
Re: connection leak in HttpMethodBase when HttpRecoverableException
is re-thrown
Posted by danch <da...@danch.com>.
Thanks for the reply.
I pulled down the release2 branch to do this testing. The branch built without my patch locks up in
'doGetConnection' if I start a multithreaded test then kill the webservices server, causing
IOExceptions as I described.
I'm rather leary of trying HEAD both because I need stable code, and because the Axis
CommonsHttpSender is written to the 2.0 apis - I'm not sure if anything significant has changed or
not. I can give this a shot and see if the current HEAD exhibits the same behavior, but I probably
won't be able to do that until tomorrow.
I'd seen bug 23137 - the patch for that seems to be for HEAD, not the release 2 branch.
Actually, I'd also seen bug 22841 - that fixes releases where an exception pops out of the close
call in releaseConnection. In my case, the problem was that releaseConnection was never called.
thanks again,
danch
Eric Johnson wrote:
> Danch,
>
> Note the bugs:
> http://issues.apache.org/bugzilla/show_bug.cgi?id=23137
> http://issues.apache.org/bugzilla/show_bug.cgi?id=22841
>
> If you can, pull the latest code from CVS, or grab a nightly build, and
> try it again, as the bug may very well be fixed already.
>
> Let us know how it goes.
>
> - Eric
>
> danch wrote:
>
>> Hello,
>> I'm using HttpClient (2.0 rc1) under Axis (using their
>> CommonsHttpSender class) and we've run into some problems with a web
>> application locking up (deadlock).
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
Re: connection leak in HttpMethodBase when HttpRecoverableException is
re-thrown
Posted by Eric Johnson <er...@tibco.com>.
Danch,
Note the bugs:
http://issues.apache.org/bugzilla/show_bug.cgi?id=23137
http://issues.apache.org/bugzilla/show_bug.cgi?id=22841
If you can, pull the latest code from CVS, or grab a nightly build, and
try it again, as the bug may very well be fixed already.
Let us know how it goes.
- Eric
danch wrote:
> Hello,
> I'm using HttpClient (2.0 rc1) under Axis (using their
> CommonsHttpSender class) and we've run into some problems with a web
> application locking up (deadlock).
>
> A little research and a test application revealed that this our
> application arrived at this condition after receiving a couple of
> exceptions from HttpMethodBase.processRequest. If processRequest
> catches an HttpRecoverableException, it attempts to determine whether
> a retry should be attempted or not, and if not (which is our case), it
> rethrows the exception, which is propogated on up the stack:
>
> } catch (HttpRecoverableException httpre) {
> if (LOG.isDebugEnabled()) {
> LOG.debug("Closing the connection.");
> }
> connection.close();
> LOG.info("Recoverable exception caught when processing request");
> // update the recoverable exception count.
> recoverableExceptionCount++;
>
> // test if this method should be retried
> if (!getMethodRetryHandler().retryMethod(
> this,
> connection,
> httpre,
> execCount,
> requestSent)
> ) {
> LOG.warn("Recoverable exception caught but
> MethodRetryHandler.retryMethod() "
> + "returned false, rethrowing exception"
> );
> throw httpre;
> }
> }
>
>
> The caller of processRequest (HttpMethodBase.execute) does not handle
> the exception, but has the following finally block:
>
> } finally {
> inExecute = false;
> // If the response has been fully processed, return the connection
> // to the pool. Use this flag, rather than other tests (like
> // responseStream == null), as subclasses, might reset the stream,
> // for example, reading the entire response into a file and then
> // setting the file as the stream.
> if (doneWithConnection) {
> ensureConnectionRelease();
> }
> }
>
>
> Note that the retry case is handled internally to
> HttpMethodBase.execute. I believe that the
> 'doneWithConnection flag should be set to true before rethrowing the
> HttpRecoverableException in the above catch block, so that the
> connection will be released properly. This change seems to have solved
> the problem we were having. Here's diff -u output
>
> --- HttpMethodBase.java 2003-09-24 13:10:10.249937500 -0500
> +++ HttpMethodBase.mine 2003-09-23 16:51:31.394929000 -0500
> @@ -2644,6 +2644,7 @@
> "Recoverable exception caught but MethodRetryHandler.retryMethod() "
> + "returned false, rethrowing exception"
> );
> + doneWithConnection = true;
> throw httpre;
> }
> }
>
>
> thanks,
> danch
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail:
> commons-httpclient-dev-help@jakarta.apache.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org