From 509b1cf62782e74709eb82b719be3ebee4a361df Mon Sep 17 00:00:00 2001 From: Noah Andrews Date: Mon, 18 Mar 2019 15:53:03 -0400 Subject: [PATCH 1/4] Replace TimerTask with ScheduledExecutorService --- .../org/java_websocket/AbstractWebSocket.java | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/java_websocket/AbstractWebSocket.java b/src/main/java/org/java_websocket/AbstractWebSocket.java index 75bce42a..6a152300 100644 --- a/src/main/java/org/java_websocket/AbstractWebSocket.java +++ b/src/main/java/org/java_websocket/AbstractWebSocket.java @@ -31,8 +31,10 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Timer; -import java.util.TimerTask; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; /** @@ -60,15 +62,15 @@ public abstract class AbstractWebSocket extends WebSocketAdapter { private boolean reuseAddr; /** - * Attribute for a timer allowing to check for lost connections - * @since 1.3.4 + * Attribute for a service that triggers lost connection checking + * @since 1.4.1 */ - private Timer connectionLostTimer; + private ScheduledExecutorService connectionLostCheckerService; /** - * Attribute for a timertask allowing to check for lost connections - * @since 1.3.4 + * Attribute for a task that checks for lost connections + * @since 1.4.1 */ - private TimerTask connectionLostTimerTask; + private ScheduledFuture connectionLostCheckerFuture; /** * Attribute for the lost connection check interval @@ -139,7 +141,7 @@ public void setConnectionLostTimeout( int connectionLostTimeout ) { */ protected void stopConnectionLostTimer() { synchronized (syncConnectionLost) { - if (connectionLostTimer != null || connectionLostTimerTask != null) { + if (connectionLostCheckerService != null || connectionLostCheckerFuture != null) { this.websocketRunning = false; log.trace("Connection lost timer stopped"); cancelConnectionLostTimer(); @@ -168,8 +170,8 @@ protected void startConnectionLostTimer() { */ private void restartConnectionLostTimer() { cancelConnectionLostTimer(); - connectionLostTimer = new Timer("WebSocketTimer"); - connectionLostTimerTask = new TimerTask() { + connectionLostCheckerService = Executors.newSingleThreadScheduledExecutor(); + Runnable connectionLostChecker = new Runnable() { /** * Keep the connections in a separate list to not cause deadlocks @@ -190,8 +192,8 @@ public void run() { connections.clear(); } }; - connectionLostTimer.scheduleAtFixedRate( connectionLostTimerTask,1000L*connectionLostTimeout , 1000L*connectionLostTimeout ); + connectionLostCheckerFuture = connectionLostCheckerService.scheduleAtFixedRate(connectionLostChecker, connectionLostTimeout, connectionLostTimeout, TimeUnit.SECONDS); } /** @@ -228,13 +230,13 @@ private void executeConnectionLostDetection(WebSocket webSocket, long current) { * @since 1.3.4 */ private void cancelConnectionLostTimer() { - if( connectionLostTimer != null ) { - connectionLostTimer.cancel(); - connectionLostTimer = null; + if( connectionLostCheckerService != null ) { + connectionLostCheckerService.shutdownNow(); + connectionLostCheckerService = null; } - if( connectionLostTimerTask != null ) { - connectionLostTimerTask.cancel(); - connectionLostTimerTask = null; + if( connectionLostCheckerFuture != null ) { + connectionLostCheckerFuture.cancel(false); + connectionLostCheckerFuture = null; } } From 065e93e8e32bc0d25144b28637b01522dde32a50 Mon Sep 17 00:00:00 2001 From: Noah Andrews Date: Tue, 9 Apr 2019 14:03:16 -0400 Subject: [PATCH 2/4] Pass Issue666Test --- .../org/java_websocket/AbstractWebSocket.java | 3 +- .../util/NamedThreadFactory.java | 47 +++++++++++++++++++ .../java_websocket/issues/Issue666Test.java | 4 +- 3 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/java_websocket/util/NamedThreadFactory.java diff --git a/src/main/java/org/java_websocket/AbstractWebSocket.java b/src/main/java/org/java_websocket/AbstractWebSocket.java index 6a152300..412b39f1 100644 --- a/src/main/java/org/java_websocket/AbstractWebSocket.java +++ b/src/main/java/org/java_websocket/AbstractWebSocket.java @@ -26,6 +26,7 @@ package org.java_websocket; import org.java_websocket.framing.CloseFrame; +import org.java_websocket.util.NamedThreadFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -170,7 +171,7 @@ protected void startConnectionLostTimer() { */ private void restartConnectionLostTimer() { cancelConnectionLostTimer(); - connectionLostCheckerService = Executors.newSingleThreadScheduledExecutor(); + connectionLostCheckerService = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("connectionLostChecker")); Runnable connectionLostChecker = new Runnable() { /** diff --git a/src/main/java/org/java_websocket/util/NamedThreadFactory.java b/src/main/java/org/java_websocket/util/NamedThreadFactory.java new file mode 100644 index 00000000..1029ffc8 --- /dev/null +++ b/src/main/java/org/java_websocket/util/NamedThreadFactory.java @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2019 Noah Andrews + * + * 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.util; + +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; + +public class NamedThreadFactory implements ThreadFactory { + private final ThreadFactory defaultThreadFactory = Executors.defaultThreadFactory(); + private final AtomicInteger threadNumber = new AtomicInteger(1); + private final String threadPrefix; + + public NamedThreadFactory(String threadPrefix) { + this.threadPrefix = threadPrefix; + } + + @Override + public Thread newThread(Runnable runnable) { + Thread thread = defaultThreadFactory.newThread(runnable); + thread.setName(threadPrefix + "-" + threadNumber); + return thread; + } +} diff --git a/src/test/java/org/java_websocket/issues/Issue666Test.java b/src/test/java/org/java_websocket/issues/Issue666Test.java index 3f0619f2..afc213bd 100644 --- a/src/test/java/org/java_websocket/issues/Issue666Test.java +++ b/src/test/java/org/java_websocket/issues/Issue666Test.java @@ -80,7 +80,7 @@ public void onStart() { } for( Thread thread : mapAfter.values() ) { String name = thread.getName(); - if( !name.startsWith( "WebSocketSelector-" ) && !name.startsWith( "WebSocketWorker-" ) && !name.equals( "WebSocketTimer" ) ) { + if( !name.startsWith( "WebSocketSelector-" ) && !name.startsWith( "WebSocketWorker-" ) && !name.startsWith( "connectionLostChecker-" ) ) { Assert.fail( "Thread not correctly named! Is: " + name ); } } @@ -145,7 +145,7 @@ public void onStart() { } for( Thread thread : mapAfter.values() ) { String name = thread.getName(); - if( !name.equals( "WebSocketTimer" ) && !name.startsWith( "WebSocketWriteThread-" ) && !name.startsWith( "WebSocketConnectReadThread-" )) { + if( !name.startsWith( "connectionLostChecker-" ) && !name.startsWith( "WebSocketWriteThread-" ) && !name.startsWith( "WebSocketConnectReadThread-" )) { Assert.fail( "Thread not correctly named! Is: " + name ); } } From e40f7950ad9548a78a30106144165879b27e6561 Mon Sep 17 00:00:00 2001 From: Noah Andrews Date: Wed, 10 Apr 2019 18:38:51 -0400 Subject: [PATCH 3/4] Use nanoTime for Lost Connection Detection --- .../org/java_websocket/AbstractWebSocket.java | 22 +++++++++---------- .../org/java_websocket/WebSocketImpl.java | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/java_websocket/AbstractWebSocket.java b/src/main/java/org/java_websocket/AbstractWebSocket.java index 412b39f1..fd0a3323 100644 --- a/src/main/java/org/java_websocket/AbstractWebSocket.java +++ b/src/main/java/org/java_websocket/AbstractWebSocket.java @@ -74,10 +74,10 @@ public abstract class AbstractWebSocket extends WebSocketAdapter { private ScheduledFuture connectionLostCheckerFuture; /** - * Attribute for the lost connection check interval + * Attribute for the lost connection check interval in nanoseconds * @since 1.3.4 */ - private int connectionLostTimeout = 60; + private long connectionLostTimeout = TimeUnit.SECONDS.toNanos(60); /** * Attribute to keep track if the WebSocket Server/Client is running/connected @@ -92,12 +92,12 @@ public abstract class AbstractWebSocket extends WebSocketAdapter { /** * Get the interval checking for lost connections * Default is 60 seconds - * @return the interval + * @return the interval in seconds * @since 1.3.4 */ public int getConnectionLostTimeout() { synchronized (syncConnectionLost) { - return connectionLostTimeout; + return (int) TimeUnit.NANOSECONDS.toSeconds(connectionLostTimeout); } } @@ -110,7 +110,7 @@ public int getConnectionLostTimeout() { */ public void setConnectionLostTimeout( int connectionLostTimeout ) { synchronized (syncConnectionLost) { - this.connectionLostTimeout = connectionLostTimeout; + this.connectionLostTimeout = TimeUnit.SECONDS.toNanos(connectionLostTimeout); if (this.connectionLostTimeout <= 0) { log.trace("Connection lost timer stopped"); cancelConnectionLostTimer(); @@ -183,9 +183,9 @@ public void run() { connections.clear(); try { connections.addAll( getConnections() ); - long current = ( System.currentTimeMillis() - ( connectionLostTimeout * 1500 ) ); + long minimumPongTime = (long) (System.nanoTime() - ( connectionLostTimeout * 1.5 )); for( WebSocket conn : connections ) { - executeConnectionLostDetection(conn, current); + executeConnectionLostDetection(conn, minimumPongTime); } } catch ( Exception e ) { //Ignore this exception @@ -194,20 +194,20 @@ public void run() { } }; - connectionLostCheckerFuture = connectionLostCheckerService.scheduleAtFixedRate(connectionLostChecker, connectionLostTimeout, connectionLostTimeout, TimeUnit.SECONDS); + connectionLostCheckerFuture = connectionLostCheckerService.scheduleAtFixedRate(connectionLostChecker, connectionLostTimeout, connectionLostTimeout, TimeUnit.NANOSECONDS); } /** * Send a ping to the endpoint or close the connection since the other endpoint did not respond with a ping * @param webSocket the websocket instance - * @param current the current time in milliseconds + * @param minimumPongTime the lowest/oldest allowable last pong time (in nanoTime) before we consider the connection to be lost */ - private void executeConnectionLostDetection(WebSocket webSocket, long current) { + private void executeConnectionLostDetection(WebSocket webSocket, long minimumPongTime) { if (!(webSocket instanceof WebSocketImpl)) { return; } WebSocketImpl webSocketImpl = (WebSocketImpl) webSocket; - if( webSocketImpl.getLastPong() < current ) { + if( webSocketImpl.getLastPong() < minimumPongTime ) { log.trace("Closing connection due to no pong received: {}", webSocketImpl); webSocketImpl.closeConnection( CloseFrame.ABNORMAL_CLOSE, "The connection was closed because the other endpoint did not respond with a pong in time. For more information check: https://github.com/TooTallNate/Java-WebSocket/wiki/Lost-connection-detection" ); } else { diff --git a/src/main/java/org/java_websocket/WebSocketImpl.java b/src/main/java/org/java_websocket/WebSocketImpl.java index 80427f81..a4066975 100644 --- a/src/main/java/org/java_websocket/WebSocketImpl.java +++ b/src/main/java/org/java_websocket/WebSocketImpl.java @@ -151,7 +151,7 @@ public class WebSocketImpl implements WebSocket { /** * Attribute, when the last pong was recieved */ - private long lastPong = System.currentTimeMillis(); + private long lastPong = System.nanoTime(); /** * Attribut to synchronize the write @@ -802,7 +802,7 @@ long getLastPong() { * Update the timestamp when the last pong was received */ public void updateLastPong() { - this.lastPong = System.currentTimeMillis(); + this.lastPong = System.nanoTime(); } /** From a91197301dd380acfb22efa23e67dfb0052f6fed Mon Sep 17 00:00:00 2001 From: Noah Andrews Date: Mon, 15 Apr 2019 11:26:02 -0400 Subject: [PATCH 4/4] Replace license --- .../util/NamedThreadFactory.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/java_websocket/util/NamedThreadFactory.java b/src/main/java/org/java_websocket/util/NamedThreadFactory.java index 1029ffc8..7d115497 100644 --- a/src/main/java/org/java_websocket/util/NamedThreadFactory.java +++ b/src/main/java/org/java_websocket/util/NamedThreadFactory.java @@ -1,26 +1,26 @@ /* - * Copyright (c) 2019 Noah Andrews + * Copyright (c) 2010-2019 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: + * 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 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. + * 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.util;