457032 - Request sent from a failed CompleteListener due to connect timeout is failed immediately.

Fixed by copying the exchange queue and failing only the exchanges
that are present in the copy.
This commit is contained in:
Simone Bordet 2015-01-08 16:56:59 +01:00
parent 9618f45ead
commit 51aafc78a4
2 changed files with 50 additions and 8 deletions

View File

@ -21,6 +21,7 @@ package org.eclipse.jetty.client;
import java.io.Closeable;
import java.io.IOException;
import java.nio.channels.AsynchronousCloseException;
import java.util.ArrayList;
import java.util.List;
import java.util.Queue;
import java.util.concurrent.RejectedExecutionException;
@ -233,9 +234,11 @@ public abstract class HttpDestination implements Destination, Closeable, Dumpabl
*/
public void abort(Throwable cause)
{
HttpExchange exchange;
// Just peek(), the abort() will remove it from the queue.
while ((exchange = exchanges.peek()) != null)
// Copy the queue of exchanges and fail only those that are queued at this moment.
// The application may queue another request from the failure/complete listener
// and we don't want to fail it immediately as if it was queued before the failure.
// The call to Request.abort() will remove the exchange from the exchanges queue.
for (HttpExchange exchange : new ArrayList<>(exchanges))
exchange.getRequest().abort(cause);
}

View File

@ -29,7 +29,6 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import javax.net.ssl.SSLEngine;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
@ -57,7 +56,6 @@ import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Ignore;
import org.junit.Test;
public class HttpClientTimeoutTest extends AbstractHttpClientServerTest
@ -301,7 +299,6 @@ public class HttpClientTimeoutTest extends AbstractHttpClientServerTest
}
}
@Ignore
@Slow
@Test
public void testConnectTimeoutFailsRequest() throws Exception
@ -333,10 +330,9 @@ public class HttpClientTimeoutTest extends AbstractHttpClientServerTest
Assert.assertNotNull(request.getAbortCause());
}
@Ignore
@Slow
@Test
public void testConnectTimeoutIsCancelledByShorterTimeout() throws Exception
public void testConnectTimeoutIsCancelledByShorterRequestTimeout() throws Exception
{
String host = "10.255.255.1";
int port = 80;
@ -368,6 +364,49 @@ public class HttpClientTimeoutTest extends AbstractHttpClientServerTest
Assert.assertNotNull(request.getAbortCause());
}
@Test
public void retryAfterConnectTimeout() throws Exception
{
final String host = "10.255.255.1";
final int port = 80;
int connectTimeout = 1000;
assumeConnectTimeout(host, port, connectTimeout);
start(new EmptyServerHandler());
client.stop();
client.setConnectTimeout(connectTimeout);
client.start();
final CountDownLatch latch = new CountDownLatch(1);
Request request = client.newRequest(host, port);
request.scheme(scheme)
.send(new Response.CompleteListener()
{
@Override
public void onComplete(Result result)
{
if (result.isFailed())
{
// Retry
client.newRequest(host, port)
.scheme(scheme)
.send(new Response.CompleteListener()
{
@Override
public void onComplete(Result result)
{
if (result.isFailed())
latch.countDown();
}
});
}
}
});
Assert.assertTrue(latch.await(333 * connectTimeout, TimeUnit.MILLISECONDS));
Assert.assertNotNull(request.getAbortCause());
}
@Test
public void testVeryShortTimeout() throws Exception
{