460905 - Make sure TimeoutCompleteListener is cancelled if the request cannot be sent.

This commit is contained in:
Simone Bordet 2015-02-26 10:55:58 +01:00
parent 7b25674aca
commit d5a6ad2345
3 changed files with 56 additions and 13 deletions

View File

@ -666,13 +666,24 @@ public class HttpRequest implements Request
@Override
public void send(Response.CompleteListener listener)
{
if (getTimeout() > 0)
TimeoutCompleteListener timeoutListener = null;
try
{
TimeoutCompleteListener timeoutListener = new TimeoutCompleteListener(this);
timeoutListener.schedule(client.getScheduler());
responseListeners.add(timeoutListener);
if (getTimeout() > 0)
{
timeoutListener = new TimeoutCompleteListener(this);
timeoutListener.schedule(client.getScheduler());
responseListeners.add(timeoutListener);
}
send(this, listener);
}
catch (Throwable x)
{
// Do not leak the scheduler task if we
// can't even start sending the request.
if (timeoutListener != null)
timeoutListener.cancel();
}
send(this, listener);
}
private void send(HttpRequest request, Response.CompleteListener listener)

View File

@ -44,21 +44,20 @@ public class TimeoutCompleteListener implements Response.CompleteListener, Runna
@Override
public void onComplete(Result result)
{
Scheduler.Task task = this.task.getAndSet(null);
if (task != null)
{
boolean cancelled = task.cancel();
if (LOG.isDebugEnabled())
LOG.debug("Cancelled (successfully: {}) timeout task {}", cancelled, task);
}
cancel();
}
public boolean schedule(Scheduler scheduler)
{
long timeout = request.getTimeout();
Scheduler.Task task = scheduler.schedule(this, timeout, TimeUnit.MILLISECONDS);
if (this.task.getAndSet(task) != null)
Scheduler.Task existing = this.task.getAndSet(task);
if (existing != null)
{
existing.cancel();
cancel();
throw new IllegalStateException();
}
if (LOG.isDebugEnabled())
LOG.debug("Scheduled timeout task {} in {} ms for {}", task, timeout, request);
return true;
@ -71,4 +70,15 @@ public class TimeoutCompleteListener implements Response.CompleteListener, Runna
LOG.debug("Executing timeout task {} for {}", task, request);
request.abort(new TimeoutException("Total timeout elapsed"));
}
public void cancel()
{
Scheduler.Task task = this.task.getAndSet(null);
if (task != null)
{
boolean cancelled = task.cancel();
if (LOG.isDebugEnabled())
LOG.debug("Cancelled (successfully: {}) timeout task {}", cancelled, task);
}
}
}

View File

@ -428,6 +428,28 @@ public class HttpClientTimeoutTest extends AbstractHttpClientServerTest
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
}
@Test
public void testTimeoutCancelledWhenSendingThrowsException() throws Exception
{
start(new EmptyServerHandler());
long timeout = 1000;
Request request = client.newRequest("bad_scheme://localhost:" + connector.getLocalPort());
request.timeout(timeout, TimeUnit.MILLISECONDS)
.send(new Response.CompleteListener()
{
@Override
public void onComplete(Result result)
{
}
});
Thread.sleep(2 * timeout);
// If the task was not cancelled, it aborted the request.
Assert.assertNull(request.getAbortCause());
}
private void assumeConnectTimeout(String host, int port, int connectTimeout) throws IOException
{
try (Socket socket = new Socket())