Skip to content

Commit

Permalink
IXWebSocketTransport::setReadyState(): Run under lock
Browse files Browse the repository at this point in the history
When setReadyState(CLOSED) is executed from two different threads (the
server's thread, detecting a close trying to receive and a separate
sending thread), there is a race where both see _readyState as non-CLOSED
and both execute the _onCloseCallback().  Worse, the server's thread might
be returning from ->run(), unsetting the callback, while the other thread
is still about to call the _onCloseCallback(), resulting in a crash
rather than "just" a duplicate invocation.

This change ensures that setReadyState() *and* the_onCloseCallback()
are executed by a single thread till completion.

I was tempted to remove the std::atomic<> for _readyState, but left it
assuming it's still good having for getReadyState().

I've first tried _readyState.exchange() which seemed promising, but still
crashed once in a while as racing threads do not wait for the
_onCloseCallback() to complete.
  • Loading branch information
awelzel committed Jan 26, 2025
1 parent 8115594 commit 80e6c4f
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 0 deletions.
2 changes: 2 additions & 0 deletions ixwebsocket/IXWebSocketTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ namespace ix

void WebSocketTransport::setReadyState(ReadyState readyState)
{
std::lock_guard<std::mutex> lock(_setReadyStateMutex);

// No state change, return
if (_readyState == readyState) return;

Expand Down
2 changes: 2 additions & 0 deletions ixwebsocket/IXWebSocketTransport.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ namespace ix

// Hold the state of the connection (OPEN, CLOSED, etc...)
std::atomic<ReadyState> _readyState;
// Mutex to prevent racing in setReadyState()
std::mutex _setReadyStateMutex;

OnCloseCallback _onCloseCallback;
std::string _closeReason;
Expand Down

0 comments on commit 80e6c4f

Please sign in to comment.