machinezone / IXWebSocket

websocket and http client and server library, with TLS support and very few dependencies
BSD 3-Clause "New" or "Revised" License
539 stars 173 forks source link

the client did not receive the close event #474

Open engun opened 1 year ago

engun commented 1 year ago

System: Windows 11 64-bit.

This lib is great. Although there were some issues encountered during use

When the server is abnormally shut down, the client did not receive the close event. It seems that the FD_CLOSE event should be taken into account in WSAWaitForMultipleEvents in IXNetSystem::poll.

bsergean commented 1 year ago

Thanks for the good words, it's possible there are some holes in the IXNetSystem::poll.

If you want to submit a PR we can try to evaluate it and merge it.

I code on macOS and Linux, so I'm not the best person to fix advanced windows problem sadly.

dacap commented 1 year ago

It's probable that there is a missing wakeUpFromPoll() call from WebSocketTransport::close() to unlock the WebSocket::stop() function which is waiting for a _thread.join() (this thread is running WebSocket::run() which calls WebSocketTransport::poll() that can be waken up only if wakeUpFromPoll() is called again):

diff --git a/ixwebsocket/IXWebSocketTransport.cpp b/ixwebsocket/IXWebSocketTransport.cpp
index ec25465..8049fb6 100644
--- a/ixwebsocket/IXWebSocketTransport.cpp
+++ b/ixwebsocket/IXWebSocketTransport.cpp
@@ -1174,7 +1174,10 @@ namespace ix
     {
         _requestInitCancellation = true;

-        if (_readyState == ReadyState::CLOSING || _readyState == ReadyState::CLOSED) return;
+        if (_readyState == ReadyState::CLOSING || _readyState == ReadyState::CLOSED) {
+            wakeUpFromPoll(SelectInterrupt::kSendRequest);
+            return;
+        }

         if (closeWireSize == 0)
         {
bsergean commented 1 year ago

@dacap If you make a PR for this change we can give it a try on CI.

Longwater1234 commented 2 months ago

This problem still persists even with latest version. Only on Windows. On MacOS it alright.