From 51b63a28a29aa05bff226dda4d6d95ab58841fe8 Mon Sep 17 00:00:00 2001 From: marci4 Date: Mon, 5 Feb 2018 17:31:40 +0100 Subject: [PATCH] NPE on already bound port Fixes #661 Test for #661 Interrupt for selectorthread to avoid zombie threads --- .../server/WebSocketServer.java | 17 ++- .../java_websocket/issues/Issue661Test.java | 137 ++++++++++++++++++ 2 files changed, 147 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/java_websocket/issues/Issue661Test.java diff --git a/src/main/java/org/java_websocket/server/WebSocketServer.java b/src/main/java/org/java_websocket/server/WebSocketServer.java index efa90d9f1..95f547996 100644 --- a/src/main/java/org/java_websocket/server/WebSocketServer.java +++ b/src/main/java/org/java_websocket/server/WebSocketServer.java @@ -253,7 +253,7 @@ public void stop( int timeout ) throws InterruptedException { wsf.close(); synchronized ( this ) { - if( selectorthread != null ) { + if( selectorthread != null && selector != null) { selector.wakeup(); selectorthread.join( timeout ); } @@ -319,12 +319,6 @@ public void run() { onStart(); } catch ( IOException ex ) { handleFatal( null, ex ); - //Shutting down WebSocketWorkers, see #222 - if( decoders != null ) { - for( WebSocketWorker w : decoders ) { - w.interrupt(); - } - } return; } try { @@ -535,6 +529,15 @@ private void handleIOException( SelectionKey key, WebSocket conn, IOException ex private void handleFatal( WebSocket conn, Exception e ) { onError( conn, e ); + //Shutting down WebSocketWorkers, see #222 + if( decoders != null ) { + for( WebSocketWorker w : decoders ) { + w.interrupt(); + } + } + if (selectorthread != null) { + selectorthread.interrupt(); + } try { stop(); } catch ( IOException e1 ) { diff --git a/src/test/java/org/java_websocket/issues/Issue661Test.java b/src/test/java/org/java_websocket/issues/Issue661Test.java new file mode 100644 index 000000000..05add1248 --- /dev/null +++ b/src/test/java/org/java_websocket/issues/Issue661Test.java @@ -0,0 +1,137 @@ +/* + * 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.java_websocket.util.ThreadCheck; +import org.junit.Rule; +import org.junit.Test; + +import java.io.OutputStream; +import java.io.PrintStream; +import java.net.BindException; +import java.net.InetSocketAddress; +import java.util.concurrent.CountDownLatch; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class Issue661Test { + + @Rule + public ThreadCheck zombies = new ThreadCheck(); + + private CountDownLatch countServerDownLatch = new CountDownLatch( 1 ); + + private boolean wasError = false; + + class TestPrintStream extends PrintStream { + public TestPrintStream( OutputStream out ) { + super( out ); + } + + @Override + public void println( Object o ) { + wasError = true; + super.println( o ); + } + } + + @Test(timeout = 2000) + public void testIssue() throws Exception { + System.setErr( new TestPrintStream( System.err ) ); + int port = SocketUtil.getAvailablePort(); + WebSocketServer server0 = new WebSocketServer( new InetSocketAddress( port ) ) { + @Override + public void onOpen( WebSocket conn, ClientHandshake handshake ) { + fail( "There should be no onOpen" ); + } + + @Override + public void onClose( WebSocket conn, int code, String reason, boolean remote ) { + fail( "There should be no onClose" ); + } + + @Override + public void onMessage( WebSocket conn, String message ) { + fail( "There should be no onMessage" ); + } + + @Override + public void onError( WebSocket conn, Exception ex ) { + fail( "There should be no onError!" ); + } + + @Override + public void onStart() { + countServerDownLatch.countDown(); + } + }; + server0.start(); + try { + countServerDownLatch.await(); + } catch ( InterruptedException e ) { + // + } + WebSocketServer server1 = new WebSocketServer( new InetSocketAddress( port ) ) { + @Override + public void onOpen( WebSocket conn, ClientHandshake handshake ) { + fail( "There should be no onOpen" ); + } + + @Override + public void onClose( WebSocket conn, int code, String reason, boolean remote ) { + fail( "There should be no onClose" ); + } + + @Override + public void onMessage( WebSocket conn, String message ) { + fail( "There should be no onMessage" ); + } + + @Override + public void onError( WebSocket conn, Exception ex ) { + if( !( ex instanceof BindException ) ) { + fail( "There should be no onError" ); + } + } + + @Override + public void onStart() { + fail( "There should be no onStart!" ); + } + }; + server1.start(); + Thread.sleep( 1000 ); + server1.stop(); + server0.stop(); + Thread.sleep( 100 ); + assertTrue( "There was an error using System.err", !wasError ); + } +}