From cb6bacb11cf84ea09e562c9d95f57f11047fc5b6 Mon Sep 17 00:00:00 2001
From: Simone Bordet <simone.bordet@gmail.com>
Date: Thu, 12 Dec 2013 16:57:03 +0100
Subject: [PATCH] 423926 - Remove code duplication in class IdleTimeout.

Removed code duplications, and also removed method close(),
unnecessary since onClose() was performing the exact same code.

Also reviewed subclasses of IdleTimeout to make sure that they always
call onClose() when they are "closed", to make sure that the timeout
does not fire and that there are no memory leaks (the scheduler
holding a reference to the timeout task, which in turn holds a
reference to the IdleTimeout instance).
---
 .../eclipse/jetty/io/AbstractEndPoint.java    |  2 +-
 .../eclipse/jetty/io/ByteArrayEndPoint.java   |  1 +
 .../org/eclipse/jetty/io/ChannelEndPoint.java |  1 +
 .../org/eclipse/jetty/io/IdleTimeout.java     | 19 ++++++++++---------
 .../eclipse/jetty/io/ssl/SslConnection.java   |  2 +-
 .../org/eclipse/jetty/io/IdleTimeoutTest.java |  5 ++---
 6 files changed, 16 insertions(+), 14 deletions(-)

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 9ec9e60afea..9ba92281d73 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
@@ -109,7 +109,7 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint
     @Override
     public void close()
     {
-        super.close();
+        onClose();
     }
 
     @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 6dea1f9c5eb..6007a60a333 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
@@ -285,6 +285,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint
     @Override
     public void close()
     {
+        super.close();
         _closed=true;
     }
 
diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ChannelEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ChannelEndPoint.java
index f69af7f14e1..11e53edd1ff 100644
--- a/jetty-io/src/main/java/org/eclipse/jetty/io/ChannelEndPoint.java
+++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ChannelEndPoint.java
@@ -108,6 +108,7 @@ public class ChannelEndPoint extends AbstractEndPoint
     @Override
     public void close()
     {
+        super.close();
         LOG.debug("close {}", this);
         try
         {
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 dd81b4656a7..11144d4ac19 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
@@ -84,14 +84,12 @@ public abstract class IdleTimeout
                 return;
 
             // old timeout is too long, so cancel it.
-            Scheduler.Task oldTimeout = _timeout.getAndSet(null);
-            if (oldTimeout != null)
-                oldTimeout.cancel();
+            deactivate();
         }
 
         // If we have a new timeout, then check and reschedule
-        if (idleTimeout > 0 && isOpen())
-            _idleTask.run();
+        if (isOpen())
+            activate();
     }
 
     /**
@@ -113,6 +111,11 @@ public abstract class IdleTimeout
     }
 
     public void onOpen()
+    {
+        activate();
+    }
+
+    private void activate()
     {
         if (_idleTimeout > 0)
             _idleTask.run();
@@ -120,12 +123,10 @@ public abstract class IdleTimeout
 
     public void onClose()
     {
-        Scheduler.Task oldTimeout = _timeout.getAndSet(null);
-        if (oldTimeout != null)
-            oldTimeout.cancel();
+        deactivate();
     }
 
-    protected void close()
+    private void deactivate()
     {
         Scheduler.Task oldTimeout = _timeout.getAndSet(null);
         if (oldTimeout != null)
diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java
index cb9f9b640f8..7f307ad1fc0 100644
--- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java
+++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java
@@ -23,7 +23,6 @@ import java.nio.ByteBuffer;
 import java.nio.channels.ClosedChannelException;
 import java.util.Arrays;
 import java.util.concurrent.Executor;
-
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLEngineResult;
 import javax.net.ssl.SSLEngineResult.HandshakeStatus;
@@ -877,6 +876,7 @@ public class SslConnection extends AbstractConnection
         @Override
         public void close()
         {
+            super.close();
             // First send the TLS Close Alert, then the FIN
             shutdownOutput();
             getEndPoint().close();
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
index d3974fc6d56..71647980131 100644
--- a/jetty-io/src/test/java/org/eclipse/jetty/io/IdleTimeoutTest.java
+++ b/jetty-io/src/test/java/org/eclipse/jetty/io/IdleTimeoutTest.java
@@ -21,10 +21,9 @@ 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.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -100,7 +99,7 @@ public class IdleTimeoutTest
             Thread.sleep(100);
             _timeout.notIdle();
         }
-        _timeout.close();
+        _timeout.onClose();
         Thread.sleep(1500);
         Assert.assertNull(_expired);
     }