From 36f87ccf45b9e47ca318cb773bc19080a6d03798 Mon Sep 17 00:00:00 2001 From: pcdv Date: Sat, 21 Oct 2017 18:48:40 +0200 Subject: [PATCH 1/4] Add junit to test classpath --- build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle b/build.gradle index b708b9f61..400637a89 100644 --- a/build.gradle +++ b/build.gradle @@ -25,6 +25,7 @@ configure(install.repositories.mavenInstaller) { dependencies { deployerJars "org.apache.maven.wagon:wagon-webdav:1.0-beta-2" + testCompile "junit:junit:4.11" } From daa52187103b40e874713a18ccb3a132607a1aac Mon Sep 17 00:00:00 2001 From: pcdv Date: Sat, 21 Oct 2017 18:55:37 +0200 Subject: [PATCH 2/4] Reproduce #580: threads still alive after client and server are stopped --- .../java_websocket/misc/ZombieThreadTest.java | 69 +++++++++++++++++++ .../org/java_websocket/util/DummyClient.java | 36 ++++++++++ .../org/java_websocket/util/DummyServer.java | 26 +++++++ .../org/java_websocket/util/ThreadCheck.java | 66 ++++++++++++++++++ 4 files changed, 197 insertions(+) create mode 100644 src/test/java/org/java_websocket/misc/ZombieThreadTest.java create mode 100644 src/test/java/org/java_websocket/util/DummyClient.java create mode 100644 src/test/java/org/java_websocket/util/DummyServer.java create mode 100644 src/test/java/org/java_websocket/util/ThreadCheck.java diff --git a/src/test/java/org/java_websocket/misc/ZombieThreadTest.java b/src/test/java/org/java_websocket/misc/ZombieThreadTest.java new file mode 100644 index 000000000..0602180dd --- /dev/null +++ b/src/test/java/org/java_websocket/misc/ZombieThreadTest.java @@ -0,0 +1,69 @@ +package org.java_websocket.misc; +import java.io.IOException; +import java.net.ServerSocket; + +import org.java_websocket.server.WebSocketServer; +import org.java_websocket.util.DummyClient; +import org.java_websocket.util.DummyServer; +import org.java_websocket.util.ThreadCheck; +import org.junit.Rule; +import org.junit.Test; + +/** + * Checks that the server leaves no active threads behind after it + * is closed. The test is ran several times because it is not systematic. + */ +public class ZombieThreadTest { + + @Rule public ThreadCheck zombies = new ThreadCheck(); + + private void runTestScenario() throws Exception { + int port = getAvailablePort(); + WebSocketServer ws = new DummyServer( port ); + + ws.start(); + Thread.sleep( 100 ); + + DummyClient clt = new DummyClient( port ); + clt.send( "foo" ); + Thread.sleep( 100 ); + clt.close(); + ws.stop(); + } + + @Test public void test1() throws Exception { + runTestScenario(); + } + + @Test public void test2() throws Exception { + runTestScenario(); + } + + @Test public void test3() throws Exception { + runTestScenario(); + } + + @Test public void test4() throws Exception { + runTestScenario(); + } + + @Test public void test5() throws Exception { + runTestScenario(); + } + + @Test public void test6() throws Exception { + runTestScenario(); + } + + private static int getAvailablePort() throws IOException { + ServerSocket srv = null; + try { + srv = new ServerSocket( 0 ); + return srv.getLocalPort(); + } finally { + if( srv != null ) { + srv.close(); + } + } + } +} diff --git a/src/test/java/org/java_websocket/util/DummyClient.java b/src/test/java/org/java_websocket/util/DummyClient.java new file mode 100644 index 000000000..c1e3a654d --- /dev/null +++ b/src/test/java/org/java_websocket/util/DummyClient.java @@ -0,0 +1,36 @@ +package org.java_websocket.util; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.java_websocket.client.WebSocketClient; +import org.java_websocket.handshake.ServerHandshake; + +/** + * Dummy client for testing. + */ +public class DummyClient extends WebSocketClient { + + private final CountDownLatch open = new CountDownLatch( 1 ); + + public DummyClient( int port ) throws URISyntaxException, InterruptedException { + super( new URI( "ws://localhost:" + port ) ); + super.connect(); + if( ! open.await( 10, TimeUnit.SECONDS ) ) { + throw new IllegalStateException( "connection timeout on " + uri ); + } + } + + @Override public void onOpen( ServerHandshake data ) { + open.countDown(); + } + + @Override public void onMessage( String msg ) {} + + @Override public void onClose( int code, String reason, boolean remote ) {} + + @Override public void onError( Exception ex ) {} + +} \ No newline at end of file diff --git a/src/test/java/org/java_websocket/util/DummyServer.java b/src/test/java/org/java_websocket/util/DummyServer.java new file mode 100644 index 000000000..bf1f09568 --- /dev/null +++ b/src/test/java/org/java_websocket/util/DummyServer.java @@ -0,0 +1,26 @@ +package org.java_websocket.util; + +import java.net.InetSocketAddress; + +import org.java_websocket.WebSocket; +import org.java_websocket.handshake.ClientHandshake; +import org.java_websocket.server.WebSocketServer; + +/** + * Simple server that just sits there, listening on a port. + */ +public class DummyServer extends WebSocketServer { + public DummyServer( int port ) { + super( new InetSocketAddress( port ) ); + } + + @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() {} +} diff --git a/src/test/java/org/java_websocket/util/ThreadCheck.java b/src/test/java/org/java_websocket/util/ThreadCheck.java new file mode 100644 index 000000000..852eac6ae --- /dev/null +++ b/src/test/java/org/java_websocket/util/ThreadCheck.java @@ -0,0 +1,66 @@ +package org.java_websocket.util; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.locks.LockSupport; + +import org.junit.Assert; +import org.junit.rules.ExternalResource; +/** + * Makes test fail if new threads are still alive after tear-down. + */ +public class ThreadCheck extends ExternalResource { + private Map map = new HashMap(); + + @Override protected void before() throws Throwable { + map = getThreadMap(); + } + + @Override protected void after() { + long time = System.currentTimeMillis(); + do { + LockSupport.parkNanos( 10000000 ); + } while ( checkZombies( true ) && System.currentTimeMillis() - time < 1000 ); + + checkZombies( false ); + } + + private boolean checkZombies( boolean testOnly ) { + Map newMap = getThreadMap(); + + int zombies = 0; + for( Thread t : newMap.values() ) { + Thread prev = map.get( t.getId() ); + if( prev == null ) { + zombies++; + if( testOnly ) + continue; + + StringBuilder b = new StringBuilder( 4096 ); + appendStack( t, b.append( "\n" ).append( t.getName() ) ); + System.err.println( b ); + } + } + if( zombies > 0 && ! testOnly ) + Assert.fail( "Found " + zombies + " zombie thread(s) " ); + + return zombies > 0; + } + + private Map getThreadMap() { + Map map = new HashMap(); + Thread[] threads = new Thread[ Thread.activeCount() * 2 ]; + int actualNb = Thread.enumerate( threads ); + for( int i = 0; i < actualNb; i++ ) { + map.put( threads[ i ].getId(), threads[ i ] ); + } + return map; + } + + private static void appendStack( Thread th, StringBuilder s ) { + StackTraceElement[] st = th.getStackTrace(); + for( int i = 0; i < st.length; i++ ) { + s.append( "\n at " ); + s.append( st[ i ] ); + } + } +} From 2e55829a1bb67aacb73cdc43f138582bcbfc4796 Mon Sep 17 00:00:00 2001 From: pcdv Date: Sun, 22 Oct 2017 18:06:32 +0200 Subject: [PATCH 3/4] Give more explicit names to created threads --- src/main/java/org/java_websocket/AbstractWebSocket.java | 2 +- src/main/java/org/java_websocket/client/WebSocketClient.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/java_websocket/AbstractWebSocket.java b/src/main/java/org/java_websocket/AbstractWebSocket.java index 1f96bda7e..0045143fe 100644 --- a/src/main/java/org/java_websocket/AbstractWebSocket.java +++ b/src/main/java/org/java_websocket/AbstractWebSocket.java @@ -108,7 +108,7 @@ protected void startConnectionLostTimer() { if (WebSocketImpl.DEBUG) System.out.println("Connection lost timer started"); cancelConnectionLostTimer(); - connectionLostTimer = new Timer(); + connectionLostTimer = new Timer(getClass().getSimpleName()+".connectionLostTimer"); connectionLostTimerTask = new TimerTask() { /** diff --git a/src/main/java/org/java_websocket/client/WebSocketClient.java b/src/main/java/org/java_websocket/client/WebSocketClient.java index bb809da18..3bba3a58d 100644 --- a/src/main/java/org/java_websocket/client/WebSocketClient.java +++ b/src/main/java/org/java_websocket/client/WebSocketClient.java @@ -229,7 +229,7 @@ public void sendPing() throws NotYetConnectedException { } public void run() { - + Thread.currentThread().setName( getClass().getSimpleName()); try { boolean isNewSocket = false; @@ -456,7 +456,7 @@ public void onFragment( Framedata frame ) { private class WebsocketWriteThread implements Runnable { @Override public void run() { - Thread.currentThread().setName( "WebsocketWriteThread" ); + Thread.currentThread().setName( WebSocketClient.this.getClass().getSimpleName() + ".WebsocketWriteThread" ); try { try { while( !Thread.interrupted() ) { From f2ec8c11e3aa9ef973a5bc8b7308c8ea4a1405dc Mon Sep 17 00:00:00 2001 From: pcdv Date: Sun, 22 Oct 2017 18:14:34 +0200 Subject: [PATCH 4/4] Cleanup --- .../java_websocket/misc/ZombieThreadTest.java | 31 ++++++------------- .../org/java_websocket/util/SocketUtil.java | 19 ++++++++++++ .../org/java_websocket/util/ThreadCheck.java | 2 +- 3 files changed, 30 insertions(+), 22 deletions(-) create mode 100644 src/test/java/org/java_websocket/util/SocketUtil.java diff --git a/src/test/java/org/java_websocket/misc/ZombieThreadTest.java b/src/test/java/org/java_websocket/misc/ZombieThreadTest.java index 0602180dd..8eb7121c2 100644 --- a/src/test/java/org/java_websocket/misc/ZombieThreadTest.java +++ b/src/test/java/org/java_websocket/misc/ZombieThreadTest.java @@ -1,10 +1,9 @@ package org.java_websocket.misc; -import java.io.IOException; -import java.net.ServerSocket; import org.java_websocket.server.WebSocketServer; import org.java_websocket.util.DummyClient; import org.java_websocket.util.DummyServer; +import org.java_websocket.util.SocketUtil; import org.java_websocket.util.ThreadCheck; import org.junit.Rule; import org.junit.Test; @@ -17,17 +16,19 @@ public class ZombieThreadTest { @Rule public ThreadCheck zombies = new ThreadCheck(); - private void runTestScenario() throws Exception { - int port = getAvailablePort(); - WebSocketServer ws = new DummyServer( port ); + private void runTestScenario() throws Exception { + WebSocketServer ws = new DummyServer( SocketUtil.getAvailablePort() ); ws.start(); - Thread.sleep( 100 ); - DummyClient clt = new DummyClient( port ); + // need to wait a bit otherwise client cannot connect + Thread.sleep( 50 ); + + DummyClient clt = new DummyClient( ws.getPort() ); clt.send( "foo" ); - Thread.sleep( 100 ); - clt.close(); + // test fails (most of the time) if we remove the closeConnection() call + // clt.close(); // not required if closeConnection() is called + clt.closeConnection( 0, "" ); ws.stop(); } @@ -54,16 +55,4 @@ private void runTestScenario() throws Exception { @Test public void test6() throws Exception { runTestScenario(); } - - private static int getAvailablePort() throws IOException { - ServerSocket srv = null; - try { - srv = new ServerSocket( 0 ); - return srv.getLocalPort(); - } finally { - if( srv != null ) { - srv.close(); - } - } - } } diff --git a/src/test/java/org/java_websocket/util/SocketUtil.java b/src/test/java/org/java_websocket/util/SocketUtil.java new file mode 100644 index 000000000..1a325b3f3 --- /dev/null +++ b/src/test/java/org/java_websocket/util/SocketUtil.java @@ -0,0 +1,19 @@ +package org.java_websocket.util; +import java.io.IOException; +import java.net.ServerSocket; +/** + * @author pcdv + */ +public class SocketUtil { + public static int getAvailablePort() throws IOException { + ServerSocket srv = null; + try { + srv = new ServerSocket( 0 ); + return srv.getLocalPort(); + } finally { + if( srv != null ) { + srv.close(); + } + } + } +} diff --git a/src/test/java/org/java_websocket/util/ThreadCheck.java b/src/test/java/org/java_websocket/util/ThreadCheck.java index 852eac6ae..9b6afebb0 100644 --- a/src/test/java/org/java_websocket/util/ThreadCheck.java +++ b/src/test/java/org/java_websocket/util/ThreadCheck.java @@ -33,7 +33,7 @@ private boolean checkZombies( boolean testOnly ) { if( prev == null ) { zombies++; if( testOnly ) - continue; + return true; StringBuilder b = new StringBuilder( 4096 ); appendStack( t, b.append( "\n" ).append( t.getName() ) );