Gottox / socket.io-java-client

Socket.IO Client Implementation in Java
MIT License
950 stars 387 forks source link

Unterminated threads from wss connections #43

Open downtownallday opened 11 years ago

downtownallday commented 11 years ago

I have been experimenting with this library for a couple of months, and it seems to work well. Thanks to those who contributed. I wanted to share a fix to a problem where the JVM will not shut down properly because of lingering non-daemon threads.

After you connect to a SSL server using the websocket transport, you will see a thread named "pool-1-thread-1" appear. If you reconnect, or connect to a different server you'll see an additional "pool-2-thread-1" and so on.

These threads don't die and are non-daemon. I have tracked the problem down to the constructor call in org.java_websocket.client.DefaultSSLWebSocketClientFactory(SLContext sslContext), which calls "Executors.newSingleThreadScheduledExecutor()". The thread created by this function is not reused and never shut down.

Here's the fix I made to WebsocketTransport.java:

diff --git "a/C:\\Users\\WxPt\\AppData\\Local\\Temp\\WebF1AB.tmp\\WebsocketTransport-8f4c6b7-left.java" "b/C:\\ExternalLibs\\sources\\socket.io-java-client\\src\\io\\socket\\WebsocketTransport.java"
index 0ec9bee..df362dd 100644
--- "a/C:\\Users\\WxPt\\AppData\\Local\\Temp\\WebF1AB.tmp\\WebsocketTransport-8f4c6b7-left.java"
+++ "b/C:\\ExternalLibs\\sources\\socket.io-java-client\\src\\io\\socket\\WebsocketTransport.java"
@@ -3,6 +3,8 @@ package io.socket;
 import java.io.IOException;
 import java.net.URI;
 import java.net.URL;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.regex.Pattern;

 import javax.net.ssl.SSLContext;
@@ -16,6 +18,7 @@ class WebsocketTransport extends WebSocketClient implements IOTransport {
     private final static Pattern PATTERN_HTTP = Pattern.compile("^http");
     public static final String TRANSPORT_NAME = "websocket";
     private IOConnection connection;
+    private ExecutorService exec;
     public static IOTransport create(URL url, IOConnection connection) {
         URI uri = URI.create(
                 PATTERN_HTTP.matcher(url.toString()).replaceFirst("ws")
@@ -30,7 +33,8 @@ class WebsocketTransport extends WebSocketClient implements IOTransport {
         this.connection = connection;
         SSLContext context = IOConnection.getSslContext();
         if("wss".equals(uri.getScheme()) && context != null) {
-           this.setWebSocketFactory(new DefaultSSLWebSocketClientFactory(context));
+           exec = Executors.newSingleThreadScheduledExecutor();
+           this.setWebSocketFactory(new DefaultSSLWebSocketClientFactory(context, exec));
         }
     }

@@ -39,6 +43,10 @@ class WebsocketTransport extends WebSocketClient implements IOTransport {
      */
     @Override
     public void disconnect() {
+       if (exec != null) {
+           exec.shutdownNow();
+           exec = null;
+       }
         try {
             this.close();
         } catch (Exception e) {
@@ -98,4 +106,4 @@ class WebsocketTransport extends WebSocketClient implements IOTransport {
         // TODO Auto-generated method stub

     }
-}
\ No newline at end of file
+}
dgthistle commented 11 years ago

I experienced the same issue and this fix worked for me. One additional note, at least on Android, if the socket disconnects with a connectivity change, be sure to use the SocketIO.reconnect call rather than disconnecting and creating a new connection. There is an issue where the backgroundTask Timer object is leaked that I have yet to solve.

veeru-as commented 11 years ago

I think even I am facing this issue. Is there anyway through which I can verify if this solves the issue or not? Basically How can list all those threads?

MasterMeyer commented 11 years ago

I did have some leaking Threads problem too, and the above fix didn't change anything for me. Updating the websocket.jar to the newest version from https://github.com/TooTallNate/Java-WebSocket did the trick for me. I had to implement one more abstract method in WebSocketTransport then.

MoxorTheOne commented 10 years ago

@veeru-as on DDMS select your process, then click Start Method Profiling (looks like a cascade arrow whith a red circle), wait a one or two seconds and press again. That will open a new view showing one row for each thread. There are other ways but I like this more because you can see wich thread is consuming the cpu across time

boopathirajaj commented 10 years ago

It helped me a great deal !!!!

Thanks a lot :)

michael-borkowski commented 10 years ago

what is the status on this? i can confirm this behaviour (j2se, not on android).