478829 - WebsocketSession not cleaned up / memory leak

 + Reducing looping references Session -> otherObj -> Session
 + Using Container LifeCycle bean management more consistently
 + All sessions are now child beans
 + A stopped session that hasn't been closed, will auto-close now
 + Using SessionListener more consistently
 + Client ConnectionManager no longer tracks Sessions
 + EventDriver stop cleans up its Session references
 + Moving all DummyConnection test classes to websocket-common:tests

6 files changed, 17 insertions, 56 deletions

diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java
index 68a2b61883..59a4d61eee 100644
--- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java
+++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java
@@ -499,6 +499,7 @@ public class WebSocketClient extends ContainerLifeCycle implements SessionListen
     {
         if (LOG.isDebugEnabled())
             LOG.debug("Session Opened: {}",session);
+        addManaged(session);
     }
     
     public void setAsyncWriteTimeout(long ms)
diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/ConnectionManager.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/ConnectionManager.java
index 580adeacbc..fb00e8f62e 100644
--- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/ConnectionManager.java
+++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/ConnectionManager.java
@@ -23,19 +23,13 @@ import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.net.URI;
 import java.nio.channels.SocketChannel;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.Locale;
-import java.util.Queue;
-import java.util.concurrent.ConcurrentLinkedQueue;
 
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
-import org.eclipse.jetty.websocket.api.StatusCode;
 import org.eclipse.jetty.websocket.client.ClientUpgradeRequest;
 import org.eclipse.jetty.websocket.client.WebSocketClient;
-import org.eclipse.jetty.websocket.common.WebSocketSession;
 import org.eclipse.jetty.websocket.common.events.EventDriver;
 
 /**
@@ -136,7 +130,6 @@ public class ConnectionManager extends ContainerLifeCycle
         return new InetSocketAddress(uri.getHost(),port);
     }
 
-    private final Queue<WebSocketSession> sessions = new ConcurrentLinkedQueue<>();
     private final WebSocketClient client;
     private WebSocketClientSelectorManager selector;
 
@@ -145,31 +138,6 @@ public class ConnectionManager extends ContainerLifeCycle
         this.client = client;
     }
 
-    public void addSession(WebSocketSession session)
-    {
-        sessions.add(session);
-    }
-
-    private void shutdownAllConnections()
-    {
-        for (WebSocketSession session : sessions)
-        {
-            if (session.getConnection() != null)
-            {
-                try
-                {
-                    session.getConnection().close(
-                            StatusCode.SHUTDOWN,
-                            "Shutdown");
-                }
-                catch (Throwable t)
-                {
-                    LOG.debug("During Shutdown All Connections",t);
-                }
-            }
-        }
-    }
-
     public ConnectPromise connect(WebSocketClient client, EventDriver driver, ClientUpgradeRequest request)
     {
         return new PhysicalConnect(client,driver,request);
@@ -189,8 +157,6 @@ public class ConnectionManager extends ContainerLifeCycle
     @Override
     protected void doStop() throws Exception
     {
-        shutdownAllConnections();
-        sessions.clear();
         super.doStop();
         removeBean(selector);
     }
@@ -200,11 +166,6 @@ public class ConnectionManager extends ContainerLifeCycle
         return selector;
     }
 
-    public Collection<WebSocketSession> getSessions()
-    {
-        return Collections.unmodifiableCollection(sessions);
-    }
-
     /**
      * Factory method for new WebSocketClientSelectorManager (used by other projects like cometd)
      * 
@@ -216,9 +177,4 @@ public class ConnectionManager extends ContainerLifeCycle
     {
         return new WebSocketClientSelectorManager(client);
     }
-
-    public void removeSession(WebSocketSession session)
-    {
-        sessions.remove(session);
-    }
 }
diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/UpgradeConnection.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/UpgradeConnection.java
index 0f0f8373db..55dbb04112 100644
--- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/UpgradeConnection.java
+++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/UpgradeConnection.java
@@ -334,7 +334,7 @@ public class UpgradeConnection extends AbstractConnection implements Connection.
         session.setOutgoingHandler(extensionStack);
         extensionStack.setNextOutgoing(connection);
 
-        session.addBean(extensionStack);
+        session.addManaged(extensionStack);
         connectPromise.getClient().addManaged(session);
 
         // Now swap out the connection
diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/WebSocketClientConnection.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/WebSocketClientConnection.java
index 4ed122b2ff..04f3f03830 100644
--- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/WebSocketClientConnection.java
+++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/WebSocketClientConnection.java
@@ -29,6 +29,7 @@ import org.eclipse.jetty.websocket.api.WriteCallback;
 import org.eclipse.jetty.websocket.api.extensions.Frame;
 import org.eclipse.jetty.websocket.api.extensions.IncomingFrames;
 import org.eclipse.jetty.websocket.client.masks.Masker;
+import org.eclipse.jetty.websocket.common.SessionListener;
 import org.eclipse.jetty.websocket.common.WebSocketFrame;
 import org.eclipse.jetty.websocket.common.WebSocketSession;
 import org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection;
@@ -41,12 +42,14 @@ public class WebSocketClientConnection extends AbstractWebSocketConnection
     private final ConnectPromise connectPromise;
     private final Masker masker;
     private final AtomicBoolean opened = new AtomicBoolean(false);
+    private final SessionListener sessionListener;
 
     public WebSocketClientConnection(EndPoint endp, Executor executor, ConnectPromise connectPromise, WebSocketPolicy policy)
     {
         super(endp,executor,connectPromise.getClient().getScheduler(),policy,connectPromise.getClient().getBufferPool());
         this.connectPromise = connectPromise;
         this.masker = connectPromise.getMasker();
+        this.sessionListener = connectPromise.getClient();
         assert (this.masker != null);
     }
 
@@ -66,8 +69,7 @@ public class WebSocketClientConnection extends AbstractWebSocketConnection
     public void onClose()
     {
         super.onClose();
-        ConnectionManager connectionManager = connectPromise.getClient().getConnectionManager();
-        connectionManager.removeSession(getSession());
+        sessionListener.onSessionClosed(getSession());
     }
 
     @Override
@@ -77,8 +79,7 @@ public class WebSocketClientConnection extends AbstractWebSocketConnection
         if (!beenOpened)
         {
             WebSocketSession session = getSession();
-            ConnectionManager connectionManager = connectPromise.getClient().getConnectionManager();
-            connectionManager.addSession(session);
+            sessionListener.onSessionOpened(session);
             connectPromise.succeeded(session);
         }
         super.onOpen();
diff --git a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/SessionTest.java b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/SessionTest.java
index 5f2b162638..c1b972995e 100644
--- a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/SessionTest.java
+++ b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/SessionTest.java
@@ -21,6 +21,7 @@ package org.eclipse.jetty.websocket.client;
 import static org.hamcrest.Matchers.*;
 
 import java.net.URI;
+import java.util.Collection;
 import java.util.Set;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
@@ -81,7 +82,8 @@ public class SessionTest
             cliSock.assertWasOpened();
             cliSock.assertNotClosed();
 
-            Assert.assertThat("client.connectionManager.sessions.size",client.getConnectionManager().getSessions().size(),is(1));
+            Collection<WebSocketSession> sessions = client.getBeans(WebSocketSession.class);
+            Assert.assertThat("client.connectionManager.sessions.size",sessions.size(),is(1));
 
             RemoteEndpoint remote = cliSock.getSession().getRemote();
             remote.sendStringByFuture("Hello World!");
diff --git a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/WebSocketClientTest.java b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/WebSocketClientTest.java
index bef3f60c65..c2653eda34 100644
--- a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/WebSocketClientTest.java
+++ b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/WebSocketClientTest.java
@@ -18,14 +18,12 @@
 
 package org.eclipse.jetty.websocket.client;
 
-import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.Matchers.*;
 
 import java.net.InetSocketAddress;
 import java.net.URI;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Future;
@@ -37,6 +35,7 @@ import org.eclipse.jetty.websocket.api.BatchMode;
 import org.eclipse.jetty.websocket.api.RemoteEndpoint;
 import org.eclipse.jetty.websocket.api.Session;
 import org.eclipse.jetty.websocket.api.UpgradeRequest;
+import org.eclipse.jetty.websocket.common.WebSocketSession;
 import org.eclipse.jetty.websocket.common.frames.TextFrame;
 import org.eclipse.jetty.websocket.common.io.FutureWriteCallback;
 import org.eclipse.jetty.websocket.common.test.BlockheadServer;
@@ -118,7 +117,8 @@ public class WebSocketClientTest
             cliSock.assertWasOpened();
             cliSock.assertNotClosed();
 
-            Assert.assertThat("client.connectionManager.sessions.size",client.getConnectionManager().getSessions().size(),is(1));
+            Collection<WebSocketSession> sessions = client.getBeans(WebSocketSession.class);
+            Assert.assertThat("client.connectionManager.sessions.size",sessions.size(),is(1));
 
             RemoteEndpoint remote = cliSock.getSession().getRemote();
             remote.sendStringByFuture("Hello World!");
@@ -164,7 +164,8 @@ public class WebSocketClientTest
             cliSock.assertWasOpened();
             cliSock.assertNotClosed();
 
-            Assert.assertThat("client.connectionManager.sessions.size",client.getConnectionManager().getSessions().size(),is(1));
+            Collection<WebSocketSession> sessions = client.getBeans(WebSocketSession.class);
+            Assert.assertThat("client.connectionManager.sessions.size",sessions.size(),is(1));
 
             FutureWriteCallback callback = new FutureWriteCallback();