You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ol...@apache.org on 2004/01/13 00:03:12 UTC
cvs commit: jakarta-commons/httpclient/src/test/org/apache/commons/httpclient TestHttpConnectionManager.java
olegk 2004/01/12 15:03:12
Modified: httpclient/src/java/org/apache/commons/httpclient
HttpMethodDirector.java
httpclient/src/test/org/apache/commons/httpclient
TestHttpConnectionManager.java
Log:
PR #25370 (exception during writeRequest leaves the connection un-released)
Contributed by Michael Becke
Reviewed by Oleg Kalnichevski
Revision Changes Path
1.13 +54 -51 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodDirector.java
Index: HttpMethodDirector.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodDirector.java,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -r1.12 -r1.13
--- HttpMethodDirector.java 12 Jan 2004 18:50:14 -0000 1.12
+++ HttpMethodDirector.java 12 Jan 2004 23:03:12 -0000 1.13
@@ -369,17 +369,17 @@
// loop until the method is successfully processed, the retryHandler
// returns false or a non-recoverable exception is thrown
- while (true) {
- execCount++;
- requestSent = false;
+ try {
+ while (true) {
+ execCount++;
+ requestSent = false;
- if (LOG.isTraceEnabled()) {
- LOG.trace("Attempt number " + execCount + " to process request");
- }
- if (!this.conn.isOpen()) {
- // this connection must be opened before it can be used
- // This has nothing to do with opening a secure tunnel
- try {
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Attempt number " + execCount + " to process request");
+ }
+ if (!this.conn.isOpen()) {
+ // this connection must be opened before it can be used
+ // This has nothing to do with opening a secure tunnel
this.conn.open();
if (this.conn.isProxied() && this.conn.isSecure()
&& !(method instanceof ConnectMethod)) {
@@ -389,49 +389,52 @@
return;
}
}
- } catch (IOException e) {
- releaseConnection = true;
- throw e;
- } catch (RuntimeException e) {
- releaseConnection = true;
- throw e;
}
- }
- try {
- method.execute(state, this.conn);
- break;
- } catch (HttpRecoverableException httpre) {
- if (LOG.isDebugEnabled()) {
+ try {
+ method.execute(state, this.conn);
+ break;
+ } catch (HttpRecoverableException httpre) {
LOG.debug("Closing the connection.");
- }
- this.conn.close();
- LOG.info("Recoverable exception caught when processing request");
- // update the recoverable exception count.
- recoverableExceptionCount++;
+ this.conn.close();
+ LOG.info("Recoverable exception caught when processing request");
+ // update the recoverable exception count.
+ recoverableExceptionCount++;
- // test if this method should be retried
- MethodRetryHandler handler = method.getMethodRetryHandler();
- if (handler == null) {
- handler = new DefaultMethodRetryHandler();
- }
- if (!handler.retryMethod(
- method,
- this.conn,
- httpre,
- execCount,
- requestSent)
- ) {
- LOG.warn(
- "Recoverable exception caught but MethodRetryHandler.retryMethod() "
- + "returned false, rethrowing exception"
- );
- // this connection can no longer be used, it has been closed
- releaseConnection = true;
- throw httpre;
+ // test if this method should be retried
+ MethodRetryHandler handler = method.getMethodRetryHandler();
+ if (handler == null) {
+ handler = new DefaultMethodRetryHandler();
+ }
+ if (!handler.retryMethod(
+ method,
+ this.conn,
+ httpre,
+ execCount,
+ requestSent)
+ ) {
+ LOG.warn(
+ "Recoverable exception caught but MethodRetryHandler.retryMethod() "
+ + "returned false, rethrowing exception"
+ );
+ throw httpre;
+ }
}
}
+ } catch (IOException e) {
+ if (this.conn.isOpen()) {
+ LOG.debug("Closing the connection.");
+ this.conn.close();
+ }
+ releaseConnection = true;
+ throw e;
+ } catch (RuntimeException e) {
+ if (this.conn.isOpen()) {
+ LOG.debug("Closing the connection.");
+ this.conn.close();
+ }
+ releaseConnection = true;
+ throw e;
}
-
}
/**
1.16 +38 -4 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnectionManager.java
Index: TestHttpConnectionManager.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnectionManager.java,v
retrieving revision 1.15
retrieving revision 1.16
diff -u -r1.15 -r1.16
--- TestHttpConnectionManager.java 6 Jan 2004 20:08:56 -0000 1.15
+++ TestHttpConnectionManager.java 12 Jan 2004 23:03:12 -0000 1.16
@@ -247,6 +247,40 @@
assertNull("connectionManager should be null", connectionManager);
}
+ public void testWriteRequestReleaseConnection() {
+
+ MultiThreadedHttpConnectionManager connectionManager = new MultiThreadedHttpConnectionManager();
+ connectionManager.getParams().setIntParameter(
+ HttpConnectionManagerParams.MAX_HOST_CONNECTIONS, 1);
+
+ HttpClient client = createHttpClient(connectionManager);
+
+ GetMethod get = new GetMethod("/") {
+ protected boolean writeRequestBody(HttpState state, HttpConnection conn)
+ throws IOException, HttpException {
+ throw new IOException("Oh no!!");
+ }
+ };
+
+ try {
+ client.executeMethod(get);
+ fail("An exception should have occurred.");
+ } catch (HttpException e) {
+ e.printStackTrace();
+ fail("HttpException should not have occurred: " + e);
+ } catch (IOException e) {
+ // expected
+ }
+
+ try {
+ connectionManager.getConnectionWithTimeout(client.getHostConfiguration(), 1);
+ } catch (ConnectTimeoutException e) {
+ e.printStackTrace();
+ fail("Connection was not released: " + e);
+ }
+
+ }
+
public void testReleaseConnection() {
MultiThreadedHttpConnectionManager connectionManager = new MultiThreadedHttpConnectionManager();
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org