Replace TimerTask with ScheduledExecutorService (TooTallNate#878)

Replace TimerTask with ScheduledExecutorService

Author: marci4

src/main/java/org/java_websocket/AbstractWebSocket.java

@@ -26,13 +26,16 @@
 packageorg.java_websocket;
 
 importorg.java_websocket.framing.CloseFrame;
+importorg.java_websocket.util.NamedThreadFactory;
 importorg.slf4j.Logger;
 importorg.slf4j.LoggerFactory;
 
 importjava.util.ArrayList;
 importjava.util.Collection;
-importjava.util.Timer;
-importjava.util.TimerTask;
+importjava.util.concurrent.Executors;
+importjava.util.concurrent.ScheduledExecutorService;
+importjava.util.concurrent.ScheduledFuture;
+importjava.util.concurrent.TimeUnit;
 
 
 /**
@@ -60,21 +63,21 @@ public abstract class AbstractWebSocket extends WebSocketAdapter{
 privatebooleanreuseAddr;
 
 /**
- * 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
  */
-privateTimerconnectionLostTimer;
+privateScheduledExecutorServiceconnectionLostCheckerService;
 /**
- * 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
  */
-privateTimerTaskconnectionLostTimerTask;
+privateScheduledFutureconnectionLostCheckerFuture;
 
 /**
- * Attribute for the lost connection check interval
+ * Attribute for the lost connection check interval in nanoseconds
  * @since 1.3.4
  */
-privateintconnectionLostTimeout = 60;
+privatelongconnectionLostTimeout = 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
  */
 publicintgetConnectionLostTimeout(){
 synchronized (syncConnectionLost){
-returnconnectionLostTimeout;
+return(int) TimeUnit.NANOSECONDS.toSeconds(connectionLostTimeout);
  }
  }
 
@@ -107,7 +110,7 @@ public int getConnectionLostTimeout(){
  */
 publicvoidsetConnectionLostTimeout( intconnectionLostTimeout ){
 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 ){
  */
 protectedvoidstopConnectionLostTimer(){
 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(){
  */
 privatevoidrestartConnectionLostTimer(){
 cancelConnectionLostTimer();
-connectionLostTimer = newTimer("WebSocketTimer");
-connectionLostTimerTask= newTimerTask(){
+connectionLostCheckerService = Executors.newSingleThreadScheduledExecutor(newNamedThreadFactory("connectionLostChecker"));
+RunnableconnectionLostChecker= newRunnable(){
 
 /**
  * Keep the connections in a separate list to not cause deadlocks
@@ -180,31 +183,31 @@ public void run(){
 connections.clear();
 try{
 connections.addAll( getConnections() );
-longcurrent = (System.currentTimeMillis() - ( connectionLostTimeout * 1500 ) );
+longminimumPongTime = (long) (System.nanoTime() - ( connectionLostTimeout * 1.5 ));
 for( WebSocketconn : connections ){
-executeConnectionLostDetection(conn, current);
+executeConnectionLostDetection(conn, minimumPongTime);
  }
  } catch ( Exceptione ){
 //Ignore this exception
  }
 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
  */
-privatevoidexecuteConnectionLostDetection(WebSocketwebSocket, longcurrent){
+privatevoidexecuteConnectionLostDetection(WebSocketwebSocket, longminimumPongTime){
 if (!(webSocketinstanceofWebSocketImpl)){
 return;
  }
 WebSocketImplwebSocketImpl = (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
  */
 privatevoidcancelConnectionLostTimer(){
-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;
  }
  }
 

src/main/java/org/java_websocket/WebSocketImpl.java

@@ -151,7 +151,7 @@ public class WebSocketImpl implements WebSocket{
 /**
  * Attribute, when the last pong was recieved
  */
-privatelonglastPong = System.currentTimeMillis();
+privatelonglastPong = System.nanoTime();
 
 /**
  * Attribut to synchronize the write
@@ -802,7 +802,7 @@ long getLastPong(){
  * Update the timestamp when the last pong was received
  */
 publicvoidupdateLastPong(){
-this.lastPong = System.currentTimeMillis();
+this.lastPong = System.nanoTime();
  }
 
 /**

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.
+ */
+
+packageorg.java_websocket.util;
+
+importjava.util.concurrent.Executors;
+importjava.util.concurrent.ThreadFactory;
+importjava.util.concurrent.atomic.AtomicInteger;
+
+publicclassNamedThreadFactoryimplementsThreadFactory{
+privatefinalThreadFactorydefaultThreadFactory = Executors.defaultThreadFactory();
+privatefinalAtomicIntegerthreadNumber = newAtomicInteger(1);
+privatefinalStringthreadPrefix;
+
+publicNamedThreadFactory(StringthreadPrefix){
+this.threadPrefix = threadPrefix;
+ }
+
+@Override
+publicThreadnewThread(Runnablerunnable){
+Threadthread = defaultThreadFactory.newThread(runnable);
+thread.setName(threadPrefix + "-" + threadNumber);
+returnthread;
+ }
+}

src/test/java/org/java_websocket/issues/Issue666Test.java

@@ -80,7 +80,7 @@ public void onStart(){
  }
 for( Threadthread : mapAfter.values() ){
 Stringname = 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( Threadthread : mapAfter.values() ){
 Stringname = 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 );
  }
  }