From 76c068dfe672fd9c64cff7623066e1c7e8b8b17f Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 11 Jan 2013 13:06:42 +1100 Subject: [PATCH] jetty-9 refined the IdleTimeout mechanism and added a unit test --- .../eclipse/jetty/io/AbstractEndPoint.java | 1 + .../eclipse/jetty/io/ByteArrayEndPoint.java | 9 -- .../org/eclipse/jetty/io/IdleTimeout.java | 28 +++- .../jetty/io/SelectChannelEndPoint.java | 12 +- .../org/eclipse/jetty/io/IdleTimeoutTest.java | 141 ++++++++++++++++++ 5 files changed, 168 insertions(+), 23 deletions(-) create mode 100644 jetty-io/src/test/java/org/eclipse/jetty/io/IdleTimeoutTest.java diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java index d0015fe4ebb..bc9ba11057d 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java @@ -94,6 +94,7 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint public void onOpen() { LOG.debug("onOpen {}",this); + super.onOpen(); } @Override diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java index 6c94ac8c658..228017a170c 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java @@ -404,14 +404,5 @@ public class ByteArrayEndPoint extends AbstractEndPoint _growOutput=growOutput; } - /* ------------------------------------------------------------ */ - @Override - public void setIdleTimeout(long idleTimeout) - { - super.setIdleTimeout(idleTimeout); - scheduleIdleTimeout(idleTimeout); - } - - } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/IdleTimeout.java b/jetty-io/src/main/java/org/eclipse/jetty/io/IdleTimeout.java index 956df9394c5..93db8b573fc 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/IdleTimeout.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/IdleTimeout.java @@ -56,7 +56,6 @@ public abstract class IdleTimeout } }; - /* ------------------------------------------------------------ */ /** * @param scheduler A scheduler used to schedule checks for the idle timeout. */ @@ -77,10 +76,27 @@ public abstract class IdleTimeout public void setIdleTimeout(long idleTimeout) { + long old=_idleTimeout; _idleTimeout = idleTimeout; + + // Do we have an old timeout + if (old>0) + { + // if the old was less than or equal to the new timeout, then nothing more to do + if (old<=_idleTimeout) + return; + + // old timeout is too long, so cancel it. + Scheduler.Task oldTimeout = _timeout.getAndSet(null); + if (oldTimeout != null) + oldTimeout.cancel(); + } + + // If we have a new timeout, then check and reschedule + if (_idleTimeout>0 && isOpen()) + _idleTask.run(); } - /* ------------------------------------------------------------ */ /** This method should be called when non-idle activity has taken place. */ public void notIdle() @@ -88,7 +104,7 @@ public abstract class IdleTimeout _idleTimestamp=System.currentTimeMillis(); } - protected void scheduleIdleTimeout(long delay) + private void scheduleIdleTimeout(long delay) { Scheduler.Task newTimeout = null; if (isOpen() && delay > 0 && _scheduler!=null) @@ -98,6 +114,12 @@ public abstract class IdleTimeout oldTimeout.cancel(); } + public void onOpen() + { + if (_idleTimeout>0) + _idleTask.run(); + } + protected void close() { Scheduler.Task oldTimeout = _timeout.getAndSet(null); diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java index 4e7925bff1f..37f6d46ae2f 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java @@ -83,13 +83,6 @@ public class SelectChannelEndPoint extends ChannelEndPoint implements SelectorMa setIdleTimeout(idleTimeout); } - @Override - public void setIdleTimeout(long idleTimeout) - { - super.setIdleTimeout(idleTimeout); - scheduleIdleTimeout(idleTimeout); - } - @Override protected boolean needsFill() { @@ -187,10 +180,7 @@ public class SelectChannelEndPoint extends ChannelEndPoint implements SelectorMa public void onOpen() { if (_open.compareAndSet(false, true)) - { super.onOpen(); - scheduleIdleTimeout(getIdleTimeout()); - } } @Override @@ -201,7 +191,7 @@ public class SelectChannelEndPoint extends ChannelEndPoint implements SelectorMa // We do a best effort to print the right toString() and that's it. try { - boolean valid = _key.isValid(); + boolean valid = _key!=null && _key.isValid(); int keyInterests = valid ? _key.interestOps() : -1; int keyReadiness = valid ? _key.readyOps() : -1; return String.format("%s{io=%d,kio=%d,kro=%d}", diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/IdleTimeoutTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/IdleTimeoutTest.java new file mode 100644 index 00000000000..783e35a419a --- /dev/null +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/IdleTimeoutTest.java @@ -0,0 +1,141 @@ +package org.eclipse.jetty.io; + + +import java.util.concurrent.TimeoutException; + +import junit.framework.Assert; + +import org.eclipse.jetty.util.thread.TimerScheduler; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class IdleTimeoutTest +{ + volatile boolean _open; + volatile TimeoutException _expired; + + TimerScheduler _timer; + IdleTimeout _timeout; + + @Before + public void setUp() throws Exception + { + _open=true; + _expired=null; + _timer=new TimerScheduler(); + _timer.start(); + _timeout=new IdleTimeout(_timer) + { + @Override + protected void onIdleExpired(TimeoutException timeout) + { + _expired=timeout; + } + + @Override + protected boolean isOpen() + { + return _open; + } + }; + _timeout.setIdleTimeout(1000); + } + + @After + public void tearDown() throws Exception + { + _open=false; + _timer.stop(); + + } + + @Test + public void testNotIdle() throws Exception + { + for (int i=0;i<20;i++) + { + Thread.sleep(100); + _timeout.notIdle(); + } + + Assert.assertNull(_expired); + } + + @Test + public void testIdle() throws Exception + { + for (int i=0;i<5;i++) + { + Thread.sleep(100); + _timeout.notIdle(); + } + Thread.sleep(1500); + Assert.assertNotNull(_expired); + } + + @Test + public void testClose() throws Exception + { + for (int i=0;i<5;i++) + { + Thread.sleep(100); + _timeout.notIdle(); + } + _timeout.close(); + Thread.sleep(1500); + Assert.assertNull(_expired); + } + + @Test + public void testClosed() throws Exception + { + for (int i=0;i<5;i++) + { + Thread.sleep(100); + _timeout.notIdle(); + } + _open=false; + Thread.sleep(1500); + Assert.assertNull(_expired); + } + + @Test + public void testShorten() throws Exception + { + for (int i=0;i<5;i++) + { + Thread.sleep(100); + _timeout.notIdle(); + } + _timeout.setIdleTimeout(100); + Thread.sleep(400); + Assert.assertNotNull(_expired); + } + + @Test + public void testLengthen() throws Exception + { + for (int i=0;i<5;i++) + { + Thread.sleep(100); + _timeout.notIdle(); + } + _timeout.setIdleTimeout(10000); + Thread.sleep(1500); + Assert.assertNull(_expired); + } + + @Test + public void testMultiple() throws Exception + { + Thread.sleep(1500); + Assert.assertNotNull(_expired); + _expired=null; + Thread.sleep(1000); + Assert.assertNotNull(_expired); + } + + + +}