329180 Spin check for Selector to stop

git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@2460 7e9141cc-0065-0410-87d8-b60c137991c4

3 files changed, 32 insertions, 49 deletions

diff --git a/VERSION.txt b/VERSION.txt
index 0ac05f3b01..66abd689de 100644
--- a/VERSION.txt
+++ b/VERSION.txt
@@ -9,6 +9,7 @@ jetty-7.2.1-SNAPSHOT
  + 328778 Improved javadoc for secure session cookies
  + 328782 allow per connection max idle time to be set
  + 328885 web overrides do not override
+ + 329180 Spin check for Selector to stop
  + JETTY-748 Prevent race close of socket by old acceptor threads
  + JETTY-1291 Extract query parameters even if POST content consumed
  + JETTY-1295 Contexts mixed up when hot-deploying on virtual hosts
diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SelectorManager.java
index 3fd226b683..8e81bac44d 100644
--- a/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SelectorManager.java
+++ b/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SelectorManager.java
@@ -419,10 +419,6 @@ public abstract class SelectorManager extends AbstractLifeCycle
                             key.attach(endpoint);
                             endpoint.schedule();
                         }
-                        else if (change instanceof Closer)
-                        {
-                            ((Closer)change).close();
-                        }
                         else if (change instanceof Runnable)
                         {
                             dispatch((Runnable)change);
@@ -790,33 +786,44 @@ public abstract class SelectorManager extends AbstractLifeCycle
         /* ------------------------------------------------------------ */
         void stop() throws Exception
         {
-            final CountDownLatch closed = new CountDownLatch(1);
-            
-            // Create runnable to close all end points
-            Closer close = new Closer()
+            // Spin for a while waiting for selector to complete 
+            // to avoid unneccessary closed channel exceptions
+            try
             {
-                public void close()
+                for (int i=0;i<100 && _selecting!=null;i++)
                 {
-                    try
+                    wakeup();
+                    Thread.sleep(1);
+                }
+            }
+            catch(Exception e)
+            {
+                Log.ignore(e);
+            }
+
+            // close endpoints and selector
+            synchronized (this)
+            {
+                for (SelectionKey key:_selector.keys())
+                {
+                    if (key==null)
+                        continue;
+                    Object att=key.attachment();
+                    if (att instanceof EndPoint)
                     {
-                        super.close();
+                        EndPoint endpoint = (EndPoint)att;
+                        try
+                        {
+                            endpoint.close();
+                        }
+                        catch(IOException e)
+                        {
+                            Log.ignore(e);
+                        }
                     }
-                    finally
-                    {
-                        closed.countDown();
-                    } 
                 }
-            };
             
-            // Try to get the selector to run the close as a change
-            addChange(close);
-
-            // if it has not been called, call it directly
-            if (!closed.await(1,TimeUnit.SECONDS))
-                close.close();   
             
-            synchronized (this)
-            {
                 _timeout.cancelAll();
                 try
                 {
@@ -847,30 +854,6 @@ public abstract class SelectorManager extends AbstractLifeCycle
             }
         }
 
-        private class Closer
-        {
-            public void close()
-            {
-                for (SelectionKey key:_selector.keys())
-                {
-                    if (key==null)
-                        continue;
-                    Object att=key.attachment();
-                    if (att instanceof EndPoint)
-                    {
-                        EndPoint endpoint = (EndPoint)att;
-                        try
-                        {
-                            endpoint.close();
-                        }
-                        catch(IOException e)
-                        {
-                            Log.ignore(e);
-                        }
-                    }
-                }
-            }   
-        }
     }
 
     /* ------------------------------------------------------------ */
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
index 32b4e6824c..e9579ccb4c 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
@@ -436,7 +436,6 @@ public class HttpConnectionTest
                 "\015\012"
              );
 
-            System.err.println(response);
             offset = checkContains(response, offset, "HTTP/1.1 500");
         }
         catch(Exception e)