From 5391ced414069f62af0ebe1e8933973aec6d4fc2 Mon Sep 17 00:00:00 2001 From: marci4 Date: Thu, 6 Dec 2018 22:19:27 +0100 Subject: [PATCH 1/2] Thread safe AbstractWebSocket --- .../org/java_websocket/AbstractWebSocket.java | 77 +++++++++++-------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/java_websocket/AbstractWebSocket.java b/src/main/java/org/java_websocket/AbstractWebSocket.java index d5019477f..b4df5e225 100644 --- a/src/main/java/org/java_websocket/AbstractWebSocket.java +++ b/src/main/java/org/java_websocket/AbstractWebSocket.java @@ -99,30 +99,39 @@ public int getConnectionLostTimeout() { * @param connectionLostTimeout the interval in seconds * @since 1.3.4 */ + + /** + * Attribute to sync on + */ + private final Object syncObject = new Object(); + + public void setConnectionLostTimeout( int connectionLostTimeout ) { - this.connectionLostTimeout = connectionLostTimeout; - if (this.connectionLostTimeout <= 0) { - log.trace( "Connection lost timer stopped" ); - cancelConnectionLostTimer(); - return; + synchronized (syncObject) { + this.connectionLostTimeout = connectionLostTimeout; + if (this.connectionLostTimeout <= 0) { + log.trace("Connection lost timer stopped"); + cancelConnectionLostTimer(); + return; + } + if (this.websocketRunning) { + log.trace("Connection lost timer restarted"); + //Reset all the pings + try { + ArrayList connections = new ArrayList(getConnections()); + WebSocketImpl webSocketImpl; + for (WebSocket conn : connections) { + if (conn instanceof WebSocketImpl) { + webSocketImpl = (WebSocketImpl) conn; + webSocketImpl.updateLastPong(); + } + } + } catch (Exception e) { + log.error("Exception during connection lost restart", e); + } + restartConnectionLostTimer(); + } } - if (this.websocketRunning) { - log.trace( "Connection lost timer restarted" ); - //Reset all the pings - try { - ArrayList connections = new ArrayList( getConnections() ); - WebSocketImpl webSocketImpl; - for( WebSocket conn : connections ) { - if( conn instanceof WebSocketImpl ) { - webSocketImpl = ( WebSocketImpl ) conn; - webSocketImpl.updateLastPong(); - } - } - } catch (Exception e) { - log.error("Exception during connection lost restart", e); - } - restartConnectionLostTimer(); - } } /** @@ -130,10 +139,12 @@ public void setConnectionLostTimeout( int connectionLostTimeout ) { * @since 1.3.4 */ protected void stopConnectionLostTimer() { - if (connectionLostTimer != null ||connectionLostTimerTask != null) { - this.websocketRunning = false; - log.trace( "Connection lost timer stopped" ); - cancelConnectionLostTimer(); + synchronized (syncObject) { + if (connectionLostTimer != null || connectionLostTimerTask != null) { + this.websocketRunning = false; + log.trace("Connection lost timer stopped"); + cancelConnectionLostTimer(); + } } } /** @@ -141,13 +152,15 @@ protected void stopConnectionLostTimer() { * @since 1.3.4 */ protected void startConnectionLostTimer() { - if (this.connectionLostTimeout <= 0) { - log.trace("Connection lost timer deactivated"); - return; + synchronized (syncObject) { + if (this.connectionLostTimeout <= 0) { + log.trace("Connection lost timer deactivated"); + return; + } + log.trace("Connection lost timer started"); + this.websocketRunning = true; + restartConnectionLostTimer(); } - log.trace("Connection lost timer started"); - this.websocketRunning = true; - restartConnectionLostTimer(); } /** From 6065a1e7bddbac5c3f7a657a7a62f48aa5ebaa21 Mon Sep 17 00:00:00 2001 From: marci4 Date: Thu, 6 Dec 2018 22:29:50 +0100 Subject: [PATCH 2/2] Added test and fixed outstanding issue --- .../org/java_websocket/AbstractWebSocket.java | 21 ++--- .../java_websocket/issues/Issue811Test.java | 91 +++++++++++++++++++ 2 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/java_websocket/issues/Issue811Test.java diff --git a/src/main/java/org/java_websocket/AbstractWebSocket.java b/src/main/java/org/java_websocket/AbstractWebSocket.java index b4df5e225..b253b8795 100644 --- a/src/main/java/org/java_websocket/AbstractWebSocket.java +++ b/src/main/java/org/java_websocket/AbstractWebSocket.java @@ -82,6 +82,10 @@ public abstract class AbstractWebSocket extends WebSocketAdapter { */ private boolean websocketRunning = false; + /** + * Attribute to sync on + */ + private final Object syncConnectionLost = new Object(); /** * Get the interval checking for lost connections * Default is 60 seconds @@ -89,7 +93,9 @@ public abstract class AbstractWebSocket extends WebSocketAdapter { * @since 1.3.4 */ public int getConnectionLostTimeout() { - return connectionLostTimeout; + synchronized (syncConnectionLost) { + return connectionLostTimeout; + } } /** @@ -99,15 +105,8 @@ public int getConnectionLostTimeout() { * @param connectionLostTimeout the interval in seconds * @since 1.3.4 */ - - /** - * Attribute to sync on - */ - private final Object syncObject = new Object(); - - public void setConnectionLostTimeout( int connectionLostTimeout ) { - synchronized (syncObject) { + synchronized (syncConnectionLost) { this.connectionLostTimeout = connectionLostTimeout; if (this.connectionLostTimeout <= 0) { log.trace("Connection lost timer stopped"); @@ -139,7 +138,7 @@ public void setConnectionLostTimeout( int connectionLostTimeout ) { * @since 1.3.4 */ protected void stopConnectionLostTimer() { - synchronized (syncObject) { + synchronized (syncConnectionLost) { if (connectionLostTimer != null || connectionLostTimerTask != null) { this.websocketRunning = false; log.trace("Connection lost timer stopped"); @@ -152,7 +151,7 @@ protected void stopConnectionLostTimer() { * @since 1.3.4 */ protected void startConnectionLostTimer() { - synchronized (syncObject) { + synchronized (syncConnectionLost) { if (this.connectionLostTimeout <= 0) { log.trace("Connection lost timer deactivated"); return; diff --git a/src/test/java/org/java_websocket/issues/Issue811Test.java b/src/test/java/org/java_websocket/issues/Issue811Test.java new file mode 100644 index 000000000..66bf6a603 --- /dev/null +++ b/src/test/java/org/java_websocket/issues/Issue811Test.java @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2010-2018 Nathan Rajlich + * + * Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +package org.java_websocket.issues; + +import org.java_websocket.WebSocket; +import org.java_websocket.handshake.ClientHandshake; +import org.java_websocket.server.WebSocketServer; +import org.java_websocket.util.SocketUtil; +import org.junit.Test; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.concurrent.CountDownLatch; + +public class Issue811Test { + private CountDownLatch countServerDownLatch = new CountDownLatch(1); + + @Test(timeout = 2000) + public void testSetConnectionLostTimeout() throws IOException, InterruptedException { + final MyWebSocketServer server = new MyWebSocketServer(new InetSocketAddress(SocketUtil.getAvailablePort())); + server.start(); + new Thread(new Runnable() { + @Override + public void run() { + while (server.getConnectionLostTimeout() == 60) { + // do nothing + } + // will never reach this statement + countServerDownLatch.countDown(); + } + }).start(); + Thread.sleep(1000); + server.setConnectionLostTimeout(20); + countServerDownLatch.await(); + } + + private static class MyWebSocketServer extends WebSocketServer { + public MyWebSocketServer(InetSocketAddress inetSocketAddress) { + super(inetSocketAddress); + } + + @Override + public void onOpen(WebSocket conn, ClientHandshake handshake) { + + } + + @Override + public void onClose(WebSocket conn, int code, String reason, boolean remote) { + + } + + @Override + public void onMessage(WebSocket conn, String message) { + + } + + @Override + public void onError(WebSocket conn, Exception ex) { + + } + + @Override + public void onStart() { + + } + } +}