Fix for #294224: HttpClient timeout setting has no effect when connecting to host.

git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@1027 7e9141cc-0065-0410-87d8-b60c137991c4
This commit is contained in:
Simone Bordet 2009-11-04 20:07:19 +00:00
parent 268926b31f
commit 1017e2235d
5 changed files with 165 additions and 25 deletions

View File

@ -13,7 +13,7 @@ jetty-7.0.1-SNAPSHOT
+ 292825 Continuations ISE rather than ignore bad transitions
+ 292546 Proactively enforce HttpClient idle timeout
+ 293222 Improved StatisticsHandler for async
+ 293506 Unable to use jconsole with Jetty when running with security manager
+ 293506 Unable to use jconsole with Jetty when running with security manager
+ 293557 Add "jad" mime mapping
+ JETTY-937 More JVM bug work arounds. Insert pause if all else fails
+ JETTY-983 Send content-length with multipart ranges
@ -29,8 +29,9 @@ jetty-7.0.1-SNAPSHOT
+ Promoted Jetty Centralized Logging from Sandbox
+ Promoted Jetty WebApp Verifier from Sandbox
+ Refactored continuation test harnessess
+ Fixed client abort asocciation
+ Fixed client abort asocciation
+ CQ-3581 jetty OSGi contribution
+ 294224 HttpClient timeout setting has no effect when connecting to host
jetty-7.0.0.v20091005 5 October 2009
291340 Race condition in onException() notifications

View File

@ -24,7 +24,6 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
@ -93,6 +92,7 @@ public class HttpClient extends HttpBuffers implements Attributes
Connector _connector;
private long _idleTimeout = 20000;
private long _timeout = 320000;
private int _connectTimeout = 75000;
private Timeout _timeoutQ = new Timeout();
private Timeout _idleTimeoutQ = new Timeout();
private Address _proxy;
@ -513,7 +513,6 @@ public class HttpClient extends HttpBuffers implements Attributes
interface Connector extends LifeCycle
{
public void startConnection(HttpDestination destination) throws IOException;
}
/**
@ -684,6 +683,22 @@ public class HttpClient extends HttpBuffers implements Attributes
_timeout = timeout;
}
/**
* @return the period in ms before timing out an attempt to connect
*/
public int getConnectTimeout()
{
return _connectTimeout;
}
/**
* @param connectTimeout the period in ms before timing out an attempt to connect
*/
public void setConnectTimeout(int connectTimeout)
{
this._connectTimeout = connectTimeout;
}
/* ------------------------------------------------------------ */
public Address getProxy()
{

View File

@ -14,9 +14,11 @@
package org.eclipse.jetty.client;
import java.io.IOException;
import java.net.SocketTimeoutException;
import java.nio.channels.SelectionKey;
import java.nio.channels.SocketChannel;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLSession;
@ -34,16 +36,18 @@ import org.eclipse.jetty.io.nio.SelectChannelEndPoint;
import org.eclipse.jetty.io.nio.SelectorManager;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.thread.Timeout;
class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector, Runnable
{
private final HttpClient _httpClient;
private final Manager _selectorManager=new Manager();
private final Timeout _connectTimer = new Timeout();
private final Map<SocketChannel, Timeout.Task> _connectingChannels = new ConcurrentHashMap<SocketChannel, Timeout.Task>();
private SSLContext _sslContext;
private Buffers _sslBuffers;
private boolean _blockingConnect;
Manager _selectorManager=new Manager();
/**
* @param httpClient
*/
@ -76,6 +80,28 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector,
{
super.doStart();
_connectTimer.setDuration(_httpClient.getConnectTimeout());
_connectTimer.setNow();
_httpClient._threadPool.dispatch(new Runnable()
{
public void run()
{
while (isRunning())
{
_connectTimer.tick(System.currentTimeMillis());
try
{
Thread.sleep(200);
}
catch (InterruptedException x)
{
Thread.currentThread().interrupt();
break;
}
}
}
});
_selectorManager.start();
final boolean direct=_httpClient.getUseDirectBuffers();
@ -124,6 +150,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector,
@Override
protected void doStop() throws Exception
{
_connectTimer.cancelAll();
_selectorManager.stop();
}
@ -136,6 +163,9 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector,
channel.configureBlocking( false );
channel.connect(address.toSocketAddress());
_selectorManager.register( channel, destination );
ConnectTimeout connectTimeout = new ConnectTimeout(channel, destination);
_connectTimer.schedule(connectTimeout);
_connectingChannels.put(channel, connectTimeout);
}
/* ------------------------------------------------------------ */
@ -193,6 +223,12 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector,
@Override
protected SelectChannelEndPoint newEndPoint(SocketChannel channel, SelectSet selectSet, SelectionKey key) throws IOException
{
// We're connected, cancel the connect timeout
Timeout.Task connectTimeout = _connectingChannels.remove(channel);
if (connectTimeout != null)
connectTimeout.cancel();
Log.debug("Channels with connection pending: {}", _connectingChannels.size());
// key should have destination at this point (will be replaced by endpoint after this call)
HttpDestination dest=(HttpDestination)key.attachment();
@ -249,4 +285,36 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector,
super.connectionFailed(channel,ex,attachment);
}
}
private class ConnectTimeout extends Timeout.Task
{
private final SocketChannel channel;
private final HttpDestination destination;
public ConnectTimeout(SocketChannel channel, HttpDestination destination)
{
this.channel = channel;
this.destination = destination;
}
@Override
protected void expired()
{
_connectingChannels.remove(channel);
if (channel.isConnectionPending())
{
Log.debug("Channel {} timed out while connecting, closing it", channel);
try
{
// This will unregister the channel from the selector
channel.close();
}
catch (IOException x)
{
Log.ignore(x);
}
destination.onConnectionFailed(new SocketTimeoutException());
}
}
}
}

View File

@ -4,18 +4,17 @@
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
// The Eclipse Public License is available at
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
// You may elect to redistribute this code under either of these licenses.
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
package org.eclipse.jetty.client;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.net.Socket;
import javax.net.SocketFactory;
import javax.net.ssl.SSLContext;
@ -55,7 +54,7 @@ class SocketConnector extends AbstractLifeCycle implements HttpClient.Connector
}
Address address = destination.isProxied() ? destination.getProxy() : destination.getAddress();
socket.connect(address.toSocketAddress());
socket.connect(address.toSocketAddress(), _httpClient.getConnectTimeout());
EndPoint endpoint=new SocketEndPoint(socket);

View File

@ -32,15 +32,14 @@ public class ConnectionTest extends TestCase
socket.bind(null);
int port=socket.getLocalPort();
socket.close();
HttpClient httpClient = new HttpClient();
httpClient.start();
CountDownLatch latch = new CountDownLatch(1);
HttpExchange exchange = new ConnectionExchange(latch);
exchange.setAddress(new Address("localhost", port));
exchange.setAddress(new Address("localhost", port));
exchange.setURI("/");
httpClient.send(exchange);
boolean passed = latch.await(1000, TimeUnit.MILLISECONDS);
@ -58,18 +57,76 @@ public class ConnectionTest extends TestCase
assertEquals(HttpExchange.STATUS_EXCEPTED, exchange.getStatus());
}
public void testConnectionTimeoutWithSocketConnector() throws Exception
{
HttpClient httpClient = new HttpClient();
httpClient.setConnectorType(HttpClient.CONNECTOR_SOCKET);
int connectTimeout = 5000;
httpClient.setConnectTimeout(connectTimeout);
httpClient.start();
try
{
CountDownLatch latch = new CountDownLatch(1);
HttpExchange exchange = new ConnectionExchange(latch);
// Using a IP address has a different behavior than using a host name
exchange.setAddress(new Address("1.2.3.4", 8080));
exchange.setURI("/");
httpClient.send(exchange);
boolean passed = latch.await(connectTimeout * 2L, TimeUnit.MILLISECONDS);
assertTrue(passed);
int status = exchange.waitForDone();
assertEquals(HttpExchange.STATUS_EXCEPTED, status);
}
finally
{
httpClient.stop();
}
}
public void testConnectionTimeoutWithSelectConnector() throws Exception
{
HttpClient httpClient = new HttpClient();
httpClient.setConnectorType(HttpClient.CONNECTOR_SELECT_CHANNEL);
int connectTimeout = 5000;
httpClient.setConnectTimeout(connectTimeout);
httpClient.start();
try
{
CountDownLatch latch = new CountDownLatch(1);
HttpExchange exchange = new ConnectionExchange(latch);
// Using a IP address has a different behavior than using a host name
exchange.setAddress(new Address("1.2.3.4", 8080));
exchange.setURI("/");
httpClient.send(exchange);
boolean passed = latch.await(connectTimeout * 2L, TimeUnit.MILLISECONDS);
assertTrue(passed);
int status = exchange.waitForDone();
assertEquals(HttpExchange.STATUS_EXCEPTED, status);
}
finally
{
httpClient.stop();
}
}
public void testIdleConnection() throws Exception
{
ServerSocket socket = new ServerSocket();
socket.bind(null);
int port=socket.getLocalPort();
HttpClient httpClient = new HttpClient();
httpClient.setIdleTimeout(700);
httpClient.start();
HttpExchange exchange = new ConnectionExchange();
exchange.setAddress(new Address("localhost", port));
exchange.setAddress(new Address("localhost", port));
exchange.setURI("/");
HttpDestination dest = httpClient.getDestination(new Address("localhost", port),false);
@ -79,15 +136,15 @@ public class ConnectionTest extends TestCase
s.getInputStream().read(buf);
assertEquals(1,dest.getConnections());
assertEquals(0,dest.getIdleConnections());
s.getOutputStream().write("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n".getBytes());
Thread.sleep(300);
assertEquals(1,dest.getConnections());
assertEquals(1,dest.getIdleConnections());
exchange = new ConnectionExchange();
exchange.setAddress(new Address("localhost", port));
exchange.setAddress(new Address("localhost", port));
exchange.setURI("/");
httpClient.send(exchange);
@ -100,16 +157,16 @@ public class ConnectionTest extends TestCase
assertEquals(1,dest.getConnections());
assertEquals(1,dest.getIdleConnections());
Thread.sleep(500);
assertEquals(0,dest.getConnections());
assertEquals(0,dest.getIdleConnections());
socket.close();
}
private class ConnectionExchange extends HttpExchange
{
private final CountDownLatch latch;
@ -118,7 +175,7 @@ public class ConnectionTest extends TestCase
{
this.latch = null;
}
private ConnectionExchange(CountDownLatch latch)
{
this.latch = latch;