Skip to content

Commit 51b63a2

Browse files
marci4marci4
committed
NPE on already bound port
Fixes TooTallNate#661 Test for TooTallNate#661 Interrupt for selectorthread to avoid zombie threads
1 parent a2b3548 commit 51b63a2

File tree

2 files changed

+147
-7
lines changed

2 files changed

+147
-7
lines changed

‎src/main/java/org/java_websocket/server/WebSocketServer.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ public void stop( int timeout ) throws InterruptedException {
253253
wsf.close();
254254

255255
synchronized ( this ) {
256-
if( selectorthread != null ) {
256+
if( selectorthread != null && selector != null) {
257257
selector.wakeup();
258258
selectorthread.join( timeout );
259259
}
@@ -319,12 +319,6 @@ public void run() {
319319
onStart();
320320
} catch ( IOException ex ) {
321321
handleFatal( null, ex );
322-
//Shutting down WebSocketWorkers, see #222
323-
if( decoders != null ) {
324-
for( WebSocketWorker w : decoders ) {
325-
w.interrupt();
326-
}
327-
}
328322
return;
329323
}
330324
try {
@@ -535,6 +529,15 @@ private void handleIOException( SelectionKey key, WebSocket conn, IOException ex
535529

536530
private void handleFatal( WebSocket conn, Exception e ) {
537531
onError( conn, e );
532+
//Shutting down WebSocketWorkers, see #222
533+
if( decoders != null ) {
534+
for( WebSocketWorker w : decoders ) {
535+
w.interrupt();
536+
}
537+
}
538+
if (selectorthread != null) {
539+
selectorthread.interrupt();
540+
}
538541
try {
539542
stop();
540543
} catch ( IOException e1 ) {

‎src/test/java/org/java_websocket/issues/Issue661Test.java‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/*
2+
* Copyright (c) 2010-2018 Nathan Rajlich
3+
*
4+
* Permission is hereby granted, free of charge, to any person
5+
* obtaining a copy of this software and associated documentation
6+
* files (the "Software"), to deal in the Software without
7+
* restriction, including without limitation the rights to use,
8+
* copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
* copies of the Software, and to permit persons to whom the
10+
* Software is furnished to do so, subject to the following
11+
* conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be
14+
* included in all copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
17+
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
18+
* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
19+
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
20+
* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
21+
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
22+
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
23+
* OTHER DEALINGS IN THE SOFTWARE.
24+
*/
25+
26+
package org.java_websocket.issues;
27+
28+
import org.java_websocket.WebSocket;
29+
import org.java_websocket.handshake.ClientHandshake;
30+
import org.java_websocket.server.WebSocketServer;
31+
import org.java_websocket.util.SocketUtil;
32+
import org.java_websocket.util.ThreadCheck;
33+
import org.junit.Rule;
34+
import org.junit.Test;
35+
36+
import java.io.OutputStream;
37+
import java.io.PrintStream;
38+
import java.net.BindException;
39+
import java.net.InetSocketAddress;
40+
import java.util.concurrent.CountDownLatch;
41+
42+
import static org.junit.Assert.assertTrue;
43+
import static org.junit.Assert.fail;
44+
45+
public class Issue661Test {
46+
47+
@Rule
48+
public ThreadCheck zombies = new ThreadCheck();
49+
50+
private CountDownLatch countServerDownLatch = new CountDownLatch( 1 );
51+
52+
private boolean wasError = false;
53+
54+
class TestPrintStream extends PrintStream {
55+
public TestPrintStream( OutputStream out ) {
56+
super( out );
57+
}
58+
59+
@Override
60+
public void println( Object o ) {
61+
wasError = true;
62+
super.println( o );
63+
}
64+
}
65+
66+
@Test(timeout = 2000)
67+
public void testIssue() throws Exception {
68+
System.setErr( new TestPrintStream( System.err ) );
69+
int port = SocketUtil.getAvailablePort();
70+
WebSocketServer server0 = new WebSocketServer( new InetSocketAddress( port ) ) {
71+
@Override
72+
public void onOpen( WebSocket conn, ClientHandshake handshake ) {
73+
fail( "There should be no onOpen" );
74+
}
75+
76+
@Override
77+
public void onClose( WebSocket conn, int code, String reason, boolean remote ) {
78+
fail( "There should be no onClose" );
79+
}
80+
81+
@Override
82+
public void onMessage( WebSocket conn, String message ) {
83+
fail( "There should be no onMessage" );
84+
}
85+
86+
@Override
87+
public void onError( WebSocket conn, Exception ex ) {
88+
fail( "There should be no onError!" );
89+
}
90+
91+
@Override
92+
public void onStart() {
93+
countServerDownLatch.countDown();
94+
}
95+
};
96+
server0.start();
97+
try {
98+
countServerDownLatch.await();
99+
} catch ( InterruptedException e ) {
100+
//
101+
}
102+
WebSocketServer server1 = new WebSocketServer( new InetSocketAddress( port ) ) {
103+
@Override
104+
public void onOpen( WebSocket conn, ClientHandshake handshake ) {
105+
fail( "There should be no onOpen" );
106+
}
107+
108+
@Override
109+
public void onClose( WebSocket conn, int code, String reason, boolean remote ) {
110+
fail( "There should be no onClose" );
111+
}
112+
113+
@Override
114+
public void onMessage( WebSocket conn, String message ) {
115+
fail( "There should be no onMessage" );
116+
}
117+
118+
@Override
119+
public void onError( WebSocket conn, Exception ex ) {
120+
if( !( ex instanceof BindException ) ) {
121+
fail( "There should be no onError" );
122+
}
123+
}
124+
125+
@Override
126+
public void onStart() {
127+
fail( "There should be no onStart!" );
128+
}
129+
};
130+
server1.start();
131+
Thread.sleep( 1000 );
132+
server1.stop();
133+
server0.stop();
134+
Thread.sleep( 100 );
135+
assertTrue( "There was an error using System.err", !wasError );
136+
}
137+
}

0 commit comments

Comments
 (0)