diff --git a/src/main/java/org/java_websocket/AbstractWebSocket.java b/src/main/java/org/java_websocket/AbstractWebSocket.java index 75bce42a..fd0a3323 100644 --- a/src/main/java/org/java_websocket/AbstractWebSocket.java +++ b/src/main/java/org/java_websocket/AbstractWebSocket.java @@ -26,13 +26,16 @@ package org.java_websocket; import org.java_websocket.framing.CloseFrame; +import org.java_websocket.util.NamedThreadFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; 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,21 +63,21 @@ 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 + * 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 @@ -89,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); } } @@ -107,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(); @@ -139,7 +142,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 +171,8 @@ protected void startConnectionLostTimer() { */ private void restartConnectionLostTimer() { cancelConnectionLostTimer(); - connectionLostTimer = new Timer("WebSocketTimer"); - connectionLostTimerTask = new TimerTask() { + connectionLostCheckerService = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("connectionLostChecker")); + Runnable connectionLostChecker = new Runnable() { /** * Keep the connections in a separate list to not cause deadlocks @@ -180,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 @@ -190,21 +193,21 @@ public void run() { connections.clear(); } }; - connectionLostTimer.scheduleAtFixedRate( connectionLostTimerTask,1000L*connectionLostTimeout , 1000L*connectionLostTimeout ); + 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 { @@ -228,13 +231,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; } } 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(); } /** 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..7d115497 --- /dev/null +++ b/src/main/java/org/java_websocket/util/NamedThreadFactory.java @@ -0,0 +1,47 @@ +/* + * 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: + * + * 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 ); } }