You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2008/03/18 20:23:00 UTC
svn commit: r638517 - in /httpcomponents/httpclient/trunk: ./
module-client/src/main/java/org/apache/http/impl/client/
module-client/src/test/java/org/apache/http/impl/client/
module-client/src/test/java/org/apache/http/impl/conn/
Author: olegk
Date: Tue Mar 18 12:22:57 2008
New Revision: 638517
URL: http://svn.apache.org/viewvc?rev=638517&view=rev
Log:
HTTPCLIENT-759: Ensure release of connections back to the connection manager on exceptions.
Contributed by Sam Berlin <sberlin at gmail.com>
Reviewed by Oleg Kalnichevski
Added:
httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java (with props)
Modified:
httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java
httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java
httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java
Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=638517&r1=638516&r2=638517&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Tue Mar 18 12:22:57 2008
@@ -1,6 +1,14 @@
Changes since 4.0 Alpha 3
-------------------
+* [HTTPCLIENT-759] Ensure release of connections back to the connection manager
+ on exceptions.
+ Contributed by Sam Berlin <sberlin at gmail.com>
+
+* [HTTPCLIENT-759] DefaultClientRequestDirector now releases connections back
+ to the connection manager on exceptions.
+ Contributed by Sam Berlin <sberlin at gmail.com>
+
* [HTTPCLIENT-758] Fixed the use of generics in AbstractHttpClient
#removeRequestInterceptorByClass and #removeResponseInterceptorByClass
Contributed by Johannes Koch <johannes.koch at fit.fraunhofer.de>
Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java?rev=638517&r1=638516&r2=638517&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java Tue Mar 18 12:22:57 2008
@@ -403,8 +403,7 @@
managedConn.close();
}
// check if we can use the same connection for the followup
- if ((managedConn != null) &&
- !followup.getRoute().equals(roureq.getRoute())) {
+ if (!followup.getRoute().equals(roureq.getRoute())) {
// the followup has a different route, release conn
//@@@ need to consume response body first?
//@@@ or let that be done in handleResponse(...)?
@@ -941,6 +940,8 @@
// we got here as the result of an exception
// no response will be returned, release the connection
managedConn = null;
+ // ensure the connection manager properly releases this connection
+ connManager.releaseConnection(mcc);
//@@@ is the connection in a re-usable state? consume response?
//@@@ for now, just shut it down
try {
Modified: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java?rev=638517&r1=638516&r2=638517&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestAllHttpClientImpl.java Tue Mar 18 12:22:57 2008
@@ -44,6 +44,7 @@
TestSuite suite = new TestSuite();
suite.addTest(TestBasicCredentialsProvider.suite());
suite.addTest(TestRequestWrapper.suite());
+ suite.addTest(TestDefaultClientRequestDirector.suite());
return suite;
}
Added: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java?rev=638517&view=auto
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java (added)
+++ httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java Tue Mar 18 12:22:57 2008
@@ -0,0 +1,194 @@
+/*
+ * $HeadURL:$
+ * $Revision:$
+ * $Date:$
+ * ====================================================================
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ */
+
+package org.apache.http.impl.client;
+
+import java.io.IOException;
+import java.net.ConnectException;
+import java.util.concurrent.TimeUnit;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.conn.ClientConnectionManager;
+import org.apache.http.conn.ConnectionPoolTimeoutException;
+import org.apache.http.conn.ManagedClientConnection;
+import org.apache.http.conn.PlainSocketFactory;
+import org.apache.http.conn.Scheme;
+import org.apache.http.conn.SchemeRegistry;
+import org.apache.http.conn.routing.HttpRoute;
+import org.apache.http.impl.conn.ClientConnAdapterMockup;
+import org.apache.http.impl.conn.SingleClientConnManager;
+import org.apache.http.localserver.ServerTestBase;
+import org.apache.http.mockup.SocketFactoryMockup;
+import org.apache.http.params.BasicHttpParams;
+import org.apache.http.params.HttpParams;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestHandler;
+
+/**
+ * Unit tests for {@link DefaultClientRequestDirector}
+ */
+public class TestDefaultClientRequestDirector extends ServerTestBase {
+
+ public TestDefaultClientRequestDirector(final String testName) throws IOException {
+ super(testName);
+ }
+
+ public static void main(String args[]) {
+ String[] testCaseName = { TestDefaultClientRequestDirector.class.getName() };
+ junit.textui.TestRunner.main(testCaseName);
+ }
+
+ public static Test suite() {
+ return new TestSuite(TestDefaultClientRequestDirector.class);
+ }
+
+ /**
+ * Tests that if a socket fails to connect, the allocated connection is
+ * properly released back to the connection manager.
+ */
+ public void testSocketConnectFailureReleasesConnection() throws Exception {
+ final ConnMan2 conMan = new ConnMan2();
+ final DefaultHttpClient client = new DefaultHttpClient(conMan, new BasicHttpParams());
+ final HttpContext context = client.getDefaultContext();
+ final HttpGet httpget = new HttpGet("http://www.example.com/a");
+
+ try {
+ client.execute(httpget, context);
+ fail("expected IOException");
+ } catch(IOException expected) {}
+
+ assertNotNull(conMan.allocatedConnection);
+ assertSame(conMan.allocatedConnection, conMan.releasedConnection);
+ }
+
+ public void testRequestFailureReleasesConnection() throws Exception {
+ this.localServer.register("*", new ThrowingService());
+
+ SchemeRegistry registry = new SchemeRegistry();
+ registry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
+
+ ConnMan3 conMan = new ConnMan3(new BasicHttpParams(), registry);
+ DefaultHttpClient client = new DefaultHttpClient(conMan, new BasicHttpParams());
+ HttpGet httpget = new HttpGet("/a");
+
+ try {
+ client.execute(getServerHttp(), httpget);
+ fail("expected IOException");
+ } catch (IOException expected) {}
+
+ assertNotNull(conMan.allocatedConnection);
+ assertSame(conMan.allocatedConnection, conMan.releasedConnection);
+ }
+
+ private static class ThrowingService implements HttpRequestHandler {
+ public void handle(
+ final HttpRequest request,
+ final HttpResponse response,
+ final HttpContext context) throws HttpException, IOException {
+ throw new IOException();
+ }
+ }
+
+ private static class ConnMan3 extends SingleClientConnManager {
+ private ManagedClientConnection allocatedConnection;
+ private ManagedClientConnection releasedConnection;
+
+ public ConnMan3(HttpParams params, SchemeRegistry schreg) {
+ super(params, schreg);
+ }
+
+ @Override
+ public ManagedClientConnection getConnection(HttpRoute route) {
+ allocatedConnection = super.getConnection(route);
+ return allocatedConnection;
+ }
+
+ @Override
+ public void releaseConnection(ManagedClientConnection conn) {
+ releasedConnection = conn;
+ super.releaseConnection(conn);
+ }
+
+
+ }
+
+ private static class ConnMan2 implements ClientConnectionManager {
+ private ManagedClientConnection allocatedConnection;
+ private ManagedClientConnection releasedConnection;
+
+ public ConnMan2() {
+ }
+
+ public void closeIdleConnections(long idletime, TimeUnit tunit) {
+ throw new UnsupportedOperationException("just a mockup");
+ }
+
+ public ManagedClientConnection getConnection(HttpRoute route)
+ throws InterruptedException {
+ throw new UnsupportedOperationException("just a mockup");
+ }
+
+ public ManagedClientConnection getConnection(HttpRoute route,
+ long timeout, TimeUnit tunit)
+ throws ConnectionPoolTimeoutException, InterruptedException {
+ allocatedConnection = new ClientConnAdapterMockup() {
+ @Override
+ public void open(HttpRoute route, HttpContext context,
+ HttpParams params) throws IOException {
+ throw new ConnectException();
+ }
+ };
+ return allocatedConnection;
+ }
+
+ public HttpParams getParams() {
+ throw new UnsupportedOperationException("just a mockup");
+ }
+
+ public SchemeRegistry getSchemeRegistry() {
+ SchemeRegistry registry = new SchemeRegistry();
+ registry.register(new Scheme("http", new SocketFactoryMockup(null), 80));
+ return registry;
+ }
+
+ public void releaseConnection(ManagedClientConnection conn) {
+ this.releasedConnection = conn;
+ }
+
+ public void shutdown() {
+ throw new UnsupportedOperationException("just a mockup");
+ }
+ }
+
+}
Propchange: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java
------------------------------------------------------------------------------
svn:keywords = Date Author Id Revision HeadURL
Propchange: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java
------------------------------------------------------------------------------
svn:mime-type = text/plain
Modified: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java?rev=638517&r1=638516&r2=638517&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java Tue Mar 18 12:22:57 2008
@@ -30,6 +30,8 @@
package org.apache.http.impl.conn;
+import java.io.IOException;
+
import org.apache.http.HttpHost;
import org.apache.http.conn.routing.HttpRoute;
import org.apache.http.params.HttpParams;
@@ -57,7 +59,7 @@
throw new UnsupportedOperationException("just a mockup");
}
- public void open(HttpRoute route, HttpContext context, HttpParams params) {
+ public void open(HttpRoute route, HttpContext context, HttpParams params) throws IOException {
throw new UnsupportedOperationException("just a mockup");
}