From fe2d969d6e6ead88274372db7e05462e98384bd6 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 12 Jun 2018 15:41:08 -0500 Subject: [PATCH] Issue #2655 - Removing closed WebSocket Session's from WebSocketClient Signed-off-by: Joakim Erdfelt --- .../jetty/websocket/api/WebSocketPolicy.java | 7 ++- .../websocket/client/WebSocketClient.java | 56 +++++++++---------- .../websocket/client/ClientCloseTest.java | 34 +++++++---- .../common/scopes/SimpleContainerScope.java | 12 ---- 4 files changed, 55 insertions(+), 54 deletions(-) diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/WebSocketPolicy.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/WebSocketPolicy.java index f210f42bd7c8..da643d738570 100644 --- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/WebSocketPolicy.java +++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/WebSocketPolicy.java @@ -144,7 +144,12 @@ public void assertValidTextMessageSize(int requestedSize) public WebSocketPolicy clonePolicy() { - WebSocketPolicy clone = new WebSocketPolicy(this.behavior); + return clonePolicy(this.behavior); + } + + public WebSocketPolicy clonePolicy(WebSocketBehavior behavior) + { + WebSocketPolicy clone = new WebSocketPolicy(behavior); clone.idleTimeout = this.idleTimeout; clone.maxTextMessageSize = this.maxTextMessageSize; clone.maxTextMessageBufferSize = this.maxTextMessageBufferSize; 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 331eb7f8fd34..472d59dc4353 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 @@ -56,8 +56,6 @@ import org.eclipse.jetty.websocket.common.WebSocketSessionFactory; import org.eclipse.jetty.websocket.common.events.EventDriverFactory; import org.eclipse.jetty.websocket.common.extensions.WebSocketExtensionFactory; -import org.eclipse.jetty.websocket.common.scopes.DelegatedContainerScope; -import org.eclipse.jetty.websocket.common.scopes.SimpleContainerScope; import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope; /** @@ -70,12 +68,16 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketCont // From HttpClient private final HttpClient httpClient; - // - private final WebSocketContainerScope containerScope; + // CDI layer + private final DecoratedObjectFactory objectFactory; + + // WebSocket Specifics + private final WebSocketPolicy policy; private final WebSocketExtensionFactory extensionRegistry; private final EventDriverFactory eventDriverFactory; private final SessionFactory sessionFactory; + // ID Generator private final int id = ThreadLocalRandom.current().nextInt(); // defaults to true for backwards compatibility @@ -112,11 +114,13 @@ public WebSocketClient(HttpClient httpClient) */ public WebSocketClient(HttpClient httpClient, DecoratedObjectFactory objectFactory) { - this.containerScope = new SimpleContainerScope(WebSocketPolicy.newClientPolicy(),new MappedByteBufferPool(),objectFactory); + this.httpClient = httpClient; - this.extensionRegistry = new WebSocketExtensionFactory(containerScope); - this.eventDriverFactory = new EventDriverFactory(containerScope); - this.sessionFactory = new WebSocketSessionFactory(containerScope); + this.policy = new WebSocketPolicy(WebSocketBehavior.CLIENT); + this.objectFactory = objectFactory; + this.extensionRegistry = new WebSocketExtensionFactory(this); + this.eventDriverFactory = new EventDriverFactory(this); + this.sessionFactory = new WebSocketSessionFactory(this); } /** @@ -231,13 +235,14 @@ private WebSocketClient(SslContextFactory sslContextFactory, Executor executor, this.httpClient.setExecutor(executor); this.httpClient.setByteBufferPool(bufferPool); addBean(this.httpClient); - - this.containerScope = new SimpleContainerScope(WebSocketPolicy.newClientPolicy(), bufferPool, objectFactory); - this.extensionRegistry = new WebSocketExtensionFactory(containerScope); + this.policy = new WebSocketPolicy(WebSocketBehavior.CLIENT); + this.objectFactory = objectFactory; + + this.extensionRegistry = new WebSocketExtensionFactory(this); - this.eventDriverFactory = new EventDriverFactory(containerScope); - this.sessionFactory = new WebSocketSessionFactory(containerScope); + this.eventDriverFactory = new EventDriverFactory(this); + this.sessionFactory = new WebSocketSessionFactory(this); } /** @@ -271,19 +276,6 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e */ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory eventDriverFactory, SessionFactory sessionFactory, HttpClient httpClient) { - WebSocketContainerScope clientScope; - if (scope.getPolicy().getBehavior() == WebSocketBehavior.CLIENT) - { - clientScope = scope; - } - else - { - // We need to wrap the scope - clientScope = new DelegatedContainerScope(WebSocketPolicy.newClientPolicy(), scope); - } - - this.containerScope = clientScope; - if(httpClient == null) { this.httpClient = HttpClientProvider.get(scope); @@ -293,8 +285,10 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e { this.httpClient = httpClient; } - - this.extensionRegistry = new WebSocketExtensionFactory(containerScope); + + this.policy = scope.getPolicy().clonePolicy(WebSocketBehavior.CLIENT); + this.objectFactory = getObjectFactory(); + this.extensionRegistry = new WebSocketExtensionFactory(this); this.eventDriverFactory = eventDriverFactory; this.sessionFactory = sessionFactory; @@ -439,7 +433,7 @@ public boolean isDispatchIO() */ public long getAsyncWriteTimeout() { - return this.containerScope.getPolicy().getAsyncWriteTimeout(); + return getPolicy().getAsyncWriteTimeout(); } public SocketAddress getBindAddress() @@ -548,7 +542,7 @@ public long getMaxTextMessageSize() @Override public DecoratedObjectFactory getObjectFactory() { - return this.containerScope.getObjectFactory(); + return this.objectFactory; } public Set getOpenSessions() @@ -559,7 +553,7 @@ public Set getOpenSessions() @Override public WebSocketPolicy getPolicy() { - return this.containerScope.getPolicy(); + return this.policy; } public Scheduler getScheduler() diff --git a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/ClientCloseTest.java b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/ClientCloseTest.java index 7fb3983e32ae..51aaf78e2853 100644 --- a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/ClientCloseTest.java +++ b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/ClientCloseTest.java @@ -18,6 +18,16 @@ package org.eclipse.jetty.websocket.client; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; + import java.io.IOException; import java.lang.reflect.Field; import java.net.SocketTimeoutException; @@ -73,15 +83,6 @@ import org.junit.Ignore; import org.junit.Test; -import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.anyOf; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertThat; - public class ClientCloseTest { private static final Logger LOG = Log.getLogger(ClientCloseTest.class); @@ -351,6 +352,8 @@ public void testHalfClose() throws Exception // client close event on ws-endpoint clientSocket.assertReceivedCloseEvent(timeout, is(StatusCode.NORMAL), containsString("From Server")); } + + assertThat("Client Open Sessions", client.getOpenSessions(), empty()); } @Ignore("Need sbordet's help here") @@ -452,6 +455,7 @@ public void testProtocolException() throws Exception // client triggers close event on client ws-endpoint clientSocket.assertReceivedCloseEvent(timeout,is(StatusCode.PROTOCOL),allOf(containsString("Invalid control frame"),containsString("length"))); + assertThat("Client Open Sessions", client.getOpenSessions(), empty()); } @Test @@ -495,6 +499,7 @@ public void testReadEOF() throws Exception containsString("Disconnected") )); } + assertThat("Client Open Sessions", client.getOpenSessions(), empty()); } @Test @@ -533,10 +538,17 @@ public void testServerNoCloseHandshake() throws Exception // client idle timeout triggers close event on client ws-endpoint assertThat("OnError Latch", clientSocket.errorLatch.await(2, TimeUnit.SECONDS), is(true)); assertThat("OnError", clientSocket.error.get(), instanceOf(TimeoutException.class)); + + // client close should occur + clientSocket.assertReceivedCloseEvent(timeout, is(StatusCode.ABNORMAL), + anyOf( + containsString("Timeout"), + containsString("Disconnected") + )); } + assertThat("Client Open Sessions", client.getOpenSessions(), empty()); } - @Ignore("Issue #2625") @Test(timeout = 5000L) public void testStopLifecycle() throws Exception { @@ -585,6 +597,7 @@ public void testStopLifecycle() throws Exception { clientSockets.get(i).assertReceivedCloseEvent(timeout, is(StatusCode.SHUTDOWN), containsString("Shutdown")); } + assertThat("Client Open Sessions", client.getOpenSessions(), empty()); } finally { @@ -638,5 +651,6 @@ public void testWriteException() throws Exception // assert - close reason message contains (write failure) clientSocket.assertReceivedCloseEvent(timeout, is(StatusCode.ABNORMAL), containsString("EOF")); } + assertThat("Client Open Sessions", client.getOpenSessions(), empty()); } } diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/scopes/SimpleContainerScope.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/scopes/SimpleContainerScope.java index 0cd32fbe585c..69e8a48e9ef7 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/scopes/SimpleContainerScope.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/scopes/SimpleContainerScope.java @@ -82,18 +82,6 @@ public SimpleContainerScope(WebSocketPolicy policy, ByteBufferPool bufferPool, E } } - @Override - protected void doStart() throws Exception - { - super.doStart(); - } - - @Override - protected void doStop() throws Exception - { - super.doStop(); - } - @Override public ByteBufferPool getBufferPool() {