jetty-9 refined the IdleTimeout mechanism and added a unit test

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

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);
-    }
-
-
 
 }

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);

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}",

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);
+    }
+    
+    
+    
+}