If you close a websocket client before it has connected, the socket remains open

[oakkitten]

if i use a workaround technique of #346, i can get the onClose call. but if i attempt to close connection before it's been established (i.e. before onOpen) call, the socket remains open long after onClose

it might be related to the fact that Threads with SocketChannels, when used with SSL, do not get interrupt()ed. in my app, i have to close the socket to interrupt such a thread.

[marci4]

Hello @oakkitten,

I used autobahn testsuite and with the current codebase the implementation passes any close test.

Have you tried the latest version of the lib?

Greetings
marci4

[marci4]

No update for 2 months!

Closing issue.
Greetings
marci4

[oakkitten]

just writing to confirm that this has been resolved. thanks for the awesome work, you rock!

[marci4]

Thank you for your feedback @oakkitten.

Happy to hear that this is solved!

[oakkitten]

i'm seeing this problem again, on 1.3.7 and 1.3.9. here's how i connect

logger.warn("CONNECTING");
try {
    client.connectBlocking()
} finally {
    logger.warn("CONNECTING END");
}

here's how i disconnect

connector.interrupt();
logger.warn("CLOSING CLIENT");
try {
    client.closeBlocking();
} catch (InterruptedException e) {
    e.printStackTrace();
}
logger.warn("CLOSING CLIENT END");

log output

12:01:51.213 W/🐶: con2 : WebSocketConnection CONNECTING

here i press disconnect

12:01:51.310 W/🐶: do2  : WebSocketConnection CLOSING CLIENT
12:01:51.646 D/🐱: con2 : SSLHandler: Server is trusted by system
12:01:51.662 W/🐶: con2 : WebSocketConnection CONNECTING END
12:01:51.664 I/System.out: write(153): {GET /endpoint HTTP/1.1
12:01:51.664 I/System.out: Connection: Upgrade
12:01:51.664 I/System.out: Host: host
12:01:51.664 I/System.out: Sec-WebSocket-Key: rBtC3umk7cczp80hnF67rw==
12:01:51.664 I/System.out: Sec-WebSocket-Version: 13
12:01:51.664 I/System.out: Upgrade: websocket
12:01:51.664 I/System.out: }
12:01:51.679 I/System.out: process(181): {HTTP/1.1 101 Switching Protocols
12:01:51.679 I/System.out: Server: nginx
12:01:51.680 I/System.out: Date: Fri, 02 Nov 2018 10:01:53 GMT
12:01:51.680 I/System.out: Connection: upgrade
12:01:51.680 I/System.out: Upgrade: websocket
12:01:51.680 I/System.out: Sec-WebSocket-Accept: aTN/IRqmTriKfyQKn2ZRySg1p24=
12:01:51.680 I/System.out: }
12:01:51.687 I/System.out: acceptHandshakeAsClient - Matching extension found: DefaultExtension
12:01:51.688 I/System.out: acceptHandshakeAsClient - Matching protocol found: 
12:01:51.688 I/System.out: open using draft: Draft_6455 extension: DefaultExtension protocol: 
12:01:51.688 I/System.out: Connection lost timer deactivated
12:01:51.689 D/🐶: WebS : WebSocketConnection WebSocket.onOpen(), readyState=OPEN

here—five minutes later—the server closes socket

12:05:31.435 D/🐶: WebS : WebSocketConnection WebSocket.onClose(code=1006, reason=)
12:05:31.441 W/🐶: do2  : WebSocketConnection CLOSING CLIENT END

if i call client.getConnection().closeConnection(1000, "foo") instead, i see:

12:18:56.839 W/🐶: do1  : WebSocketConnection CLOSING CLIENT
12:18:56.840 D/🐶: do1  : WebSocketConnection WebSocket.onClose(code=1000, reason=foo)
12:18:56.840 W/🐶: do1  : WebSocketConnection CLOSING CLIENT END
12:18:57.176 D/🐱: con1 : SSLHandler: Server is trusted by system
12:18:57.193 W/🐶: con1 : WebSocketConnection CONNECTING END
12:18:57.197 I/System.out: write(153): {GET /endpoint HTTP/1.1
12:18:57.197 I/System.out: Connection: Upgrade
12:18:57.197 I/System.out: Host: host
12:18:57.197 I/System.out: Sec-WebSocket-Key: sLr34+rlxN1g7QrVyoEy8A==
12:18:57.197 I/System.out: Sec-WebSocket-Version: 13
12:18:57.197 I/System.out: Upgrade: websocket
12:18:57.197 I/System.out: }

the sockets remain open as well

i see that my server receives 7 bytes when closeBlocking() is called, but it doesn't send anything in return.

[marci4]

Hello @oakkitten,

why do you interrupt the connector?

Could you open a separate issue with an example code so I can take a look at this?

Thank you!
Best regards,
marci4

[oakkitten]

why do you interrupt the connector?

actually my immediate problem was some code that must've been added by mistake in the development process. since the library doesn't accept socket factories, i was calling setSocket() on the client before actually connecting, and somehow forgot to check if the thread was interrupted at that point. so the main problem is resolved now.

as to why i'm calling interrupted in the first place, well. it's just there to make sure the connections are closed immediately. on my device, i see that it takes up to 3 minutes to for client.close() to make client.connectBlocking() to return (rarely, but still, and i do set connectTimeout), and when it does, the exception looks like something imposed by the OS, and i'm not sure if i can do anything save interrupt:

javax.net.ssl.SSLHandshakeException: SSL handshake aborted: ssl=0x79bfe3adc0: I/O error during system call, Connection timed out
    at com.android.org.conscrypt.NativeCrypto.SSL_do_handshake(Native Method)
    at com.android.org.conscrypt.SslWrapper.doHandshake(SslWrapper.java:374)
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.startHandshake(ConscryptFileDescriptorSocket.java:217)
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.waitForHandshake(ConscryptFileDescriptorSocket.java:468)
    at com.android.org.conscrypt.ConscryptFileDescriptorSocket.getInputStream(ConscryptFileDescriptorSocket.java:431)
    at org.java_websocket.client.WebSocketClient.run(WebSocketClient.java:269)
    at java.lang.Thread.run(Thread.java:764)

[oakkitten]

while i was poking around emulating slow connections, i also managed to get this:

WebSocket.onError(NullPointerException: Attempt to invoke interface method 'boolean org.java_websocket.handshake.ClientHandshake.hasFieldValue(java.lang.String)' on a null object reference)

[marci4]

Hello @oakkitten,

socket factories will be added for WebSocketClient in 1.4.0 (See #770).

Can you reproduce the slow connection close please for me?

Looking at the null pointer, this should not happen, so I would be happy if you can provide an example application for this as well!

Best regards,
marci4

[oakkitten]

yeah i'm waiting for 1.4.0 to be released

since i only do java on android, it might be a while before i can make some minimal demo. but it's still easy to replicate the issue if you simulate slow connection and server disappearance.

i used # tc qdisc add dev eth0 root netem delay 1000ms on the server to slow down the connection. this makes the client take about 4-5 seconds to establish the connection. so i wait for about 4 seconds and call close, and about 1 time in 5, if i do it at the right time, the client fails to stop connecting in a specified time frame. then, if needed, execute # iptables -A OUTPUT -p tcp --sport 443 -j DROP to make the server silently “disappear”.

i caught the NullPointerException in the same way, but it happened only once; didn't try reproducing it, might be very rare

[marci4]

Hello @oakkitten,

as you may see in my activity lately I am currently really busy with my normal job and real life in general. Therefore I simple do not have the time to reproduce such issues on my own and simply depend on others to provide a simple reproduction example.

On the other hand you can simple fix it and open a PR for the issues you found.

Best regards,
marci4

[oakkitten]

ok bear with me here. by now i'm not really sure how the disconnection works at all.

let's ignore WebSocketClient.close() for the time being since my server seems to be ignoring closing frames. WebSocketClient.closeConnection(1000, "foo") seems to be the one method i should be using.

now, this method ultimately calls WebSocketImpl.closeConnection(...), which checks if readyState isn't CLOSED, and then proceeds to call this.wsl.onWebsocketClose(...), which interrupts the writeThread, which ultimately calls closeSocket(), which closes the socket (why does it check if the socket exists?)

but what if WebSocketImpl.closeConnection(...) is called before we are actually connected, that is, during the execution of the following method? before we reach the creation of the writeThread, engine.readyState might already be CLOSED. the writeThread is started anyway, but it's fine as we are calling engine.eot(), right? but that method calls the same method WebSocketImpl.closeConnection(...), which does nothing because it thinks that the connection is already closed. so both the writeThread and the socket remain open.

(btw, i'm not sure but can't these calls block as well?)

[oakkitten]

i made a test here, it's very simple and should demonstrate what i'm talking about

[marci4]

Hello @oakkitten,

thank you very very much for this test!
It is simple perfect :)

I will take a look at this and will get back to you.

Best regards,
marci4

[marci4]

Hello @oakkitten,

with the help of your unit test I created a patch. What do you think of the changes?
BTW could you open a PR for your unit test?

Best regards,
marci4

[oakkitten]

if i read this right, you are interrupting writeThread and setting it to null on WebSocketConnectReadThread, but in some other places like here it can get accessed from another thread which can lead to the NPE. and there's still the issue of ostream.write()/flush() blocking. also, it would be useful not to actually connect or create any threads if the close* methods were called soon enough.

but let's talk about close() meanwhile. it only proceeds if writeThread is non-null, so in case of it being run shortly after connect(), it will do nothing. moreover, if you call closeBlocking() in the same manner, if will potentially block forever—regardless of whether you call connect or not!

but let's look at reconnect(), which is non-blocking, according to the documentation. it calls reset, which calls... closeBlocking()!

these are just the things i can spot on the very surface. the test is only so much useful; one probably should test this all with some random delays, maybe using some special real life mimicking environment?

there's just so much spaghetti in the code, i don't know if there is a way to fix it in a sane way. with many potential threads calling all the methods and very little synchronization, it's just so hard to reason about what can potentially happen

[marci4]

Maybe someone just needs to take his time and rewrite everything from the ground of.
Maybe that is the only solution....
But as far as I can tell, I will not have the time for this what so ever.

[marci4]

Maybe https://github.com/marci4/Java-WebSocket-Dev/tree/readystate is a start

[latinosamuel]

Hello,
I have the same problem.
I would like to close the wbesocketclient connection before connecting and I am not able to resolve the problem.
Is there any temporary solution to solve the previous problem?
Thank You

[marci4]

Hello @latinosamuel,

no this issue awaits your pull request!

Best regards,
Marcel

[latinosamuel]

Hello @marci4 ,
Ok.
Is there any pull request forecast?

Thank You.

[marci4]

Hello @marci4 ,
Ok.
Is there any pull request forecast?

Thank You.

If the mistake bothers you, please fix it.