Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSimone Bordet2015-02-26 09:55:58 +0000
committerSimone Bordet2015-02-26 09:56:19 +0000
commitd5a6ad23459d601e94f07a397a96d75c40179095 (patch)
tree3744e358eca69606ca8f16206daa546cd819e67f
parent7b25674acaf2410430a26dbadb039c13cb40482a (diff)
downloadorg.eclipse.jetty.project-d5a6ad23459d601e94f07a397a96d75c40179095.tar.gz
org.eclipse.jetty.project-d5a6ad23459d601e94f07a397a96d75c40179095.tar.xz
org.eclipse.jetty.project-d5a6ad23459d601e94f07a397a96d75c40179095.zip
460905 - Make sure TimeoutCompleteListener is cancelled if the request cannot be sent.
-rw-r--r--jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java21
-rw-r--r--jetty-client/src/main/java/org/eclipse/jetty/client/TimeoutCompleteListener.java26
-rw-r--r--jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java22
3 files changed, 56 insertions, 13 deletions
diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java
index 2dd02292c5..79f7504802 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java
@@ -666,13 +666,24 @@ public class HttpRequest implements Request
@Override
public void send(Response.CompleteListener listener)
{
- if (getTimeout() > 0)
+ TimeoutCompleteListener timeoutListener = null;
+ try
+ {
+ if (getTimeout() > 0)
+ {
+ timeoutListener = new TimeoutCompleteListener(this);
+ timeoutListener.schedule(client.getScheduler());
+ responseListeners.add(timeoutListener);
+ }
+ send(this, listener);
+ }
+ catch (Throwable x)
{
- TimeoutCompleteListener timeoutListener = new TimeoutCompleteListener(this);
- timeoutListener.schedule(client.getScheduler());
- responseListeners.add(timeoutListener);
+ // 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)
diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/TimeoutCompleteListener.java b/jetty-client/src/main/java/org/eclipse/jetty/client/TimeoutCompleteListener.java
index 00f2de7897..45b9a8178b 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/TimeoutCompleteListener.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/TimeoutCompleteListener.java
@@ -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);
+ }
+ }
}
diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java
index df347c4bc5..971ebf75b7 100644
--- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java
+++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java
@@ -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())

Back to the top