Endless loop in client upon close() when using wss

[downtownallday]

First, thanks so much for creating the library, it's really useful. I am using this via gottox/socket-io (client) and have run into this nasty problem of an endless loop in SSLSocketChannel2.java when closing a connection (maybe after the other side closed first). A couple of these failures is all it takes to pin the CPU at 100% as the threads that execute these loops pile up.

I doubt this is the correct fix, but what I did was change the following in SSLSocketChannel2.readRemaining(), which seems to work.

        if( inCrypt.hasRemaining() ) {
            unwrap();
            int amount = transfereTo( inData, dst );
>>>         if (engineStatus == SSLEngineResult.Status.CLOSED) {
>>>             return -1;
>>>         }
            if( amount > 0 )
                return amount;
        }

EDIT by @Davidisudadi: formatted code

[downtownallday]

Here is another endless loop that pegs the CPU. Not sure what triggers it, but this seems to fix the problem in SSLSocketChannel2.write(). I added these 3 lines just after "int num = socketChannel.write( wrap( src ) );"

    if (engineResult.getStatus() == SSLEngineResult.Status.CLOSED) {
        throw new EOFException("Connection is closed");
    }

[Davidiusdadi]

I am very sorry that i respond so late.
Your fix looks good - if you created a pull request i would merge immediately.
I will do something about this issue as soon as i have time.

@downtownallday Could you either create a pull request, post a patch file or edit your post so that it becomes clear what modification you made to fix the bug.

@temka1234 it would be nice to know if @downtownallday fix resolves your problem too.

[temka1234]

I'm new in git and don't know how to create pull request. There is no "Pull Request" button in panel.

@downtownallday fix resolves my problem.

[Davidiusdadi]

@temka1234 In oder to create a pull request you have to

1. create a fork using the fork button
2. checkout the code using git
3. commit the changes you did
4. push those changes to your fork
5. then there is a pull request button on the top

That sounds complicated but:

• https://help.github.com/ really helps with those first steps
• knowing git is cool and really beneficial in the longterm
• if your don't want to memorize all these commands you can use one of the great graphical git frontends
  • like http://code.google.com/p/tortoisegit/ on windows

[sghosh326]

Thanks for the library, it is very useful for us. We are building a peer to peer messaging layer over web sockets with a number of components communicating with each other. We are using Tomcat WebSocketServlet for the web socket server end and WebSocketClient from this project for the client (I have version 5d4dead). Each of these components are tomcat webapps and embed the one or more WebSocketClient instances to talk to the other components. So we ran into this CPU usage maxing out as well. I have also been tracking these issues to SSLSocketChannel2.
There are thread that are not getting shutdown cleanly, resulting in memory leaks when we shutdown tomcat or undeploy these webapps, which prevent the tomcat process from cleanly shutting down. My shutdown sequence involves calling WebSocket.closeConnection(CloseFrame.GOING_AWAY, "");
for each WebSocketClient instance.
So I have made 2 changes to the code base to cleanly release the threads and prevent memory leaks and this CPU maxing out.
The first one to SSLSocketChannel2.close() method, to add this:

    public void close() throws IOException {
        sslEngine.closeOutbound();
        sslEngine.getSession().invalidate();
        if( socketChannel.isOpen() )
            socketChannel.write( wrap( emptybuffer ) );// FIXME what if not all bytes can be written
        socketChannel.close();
>>      exec.shutdownNow();
    }

The memory leaks are cause by the threads in ExecutorService not being shut down cleanly. I have tried both exec.shutdown() and exec.shutdownNow(), I think either of them are fine. This is definitely needed in my scenario even with @downtownallday's changes

I also made a change to WebSocketClient.onWebsocketClose, I am less sure if this one is needed, I set a name to the writethread that I could use to identify it and did see leaks resulting from that thread not being shutdown cleanly. This might be address by @downtownallday's changes as well. Here is the change, I feel it would be good to have as protective measure:

    public final void onWebsocketClose( WebSocket conn, int code, String reason, boolean remote ) {
        connectLatch.countDown();
        closeLatch.countDown();
        if( readthread != null )
            readthread.interrupt();
>>      if (writethread != null) 
>>          writethread.interrupt();
        onClose( code, reason, remote );
    }

Thanks,
-Sandip

[ekudel]

Please merge the pull request #276

[danpe]

👍
Please merge