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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]