From edac7a0171ea061f9962b3bb8c027e3c47e9623d Mon Sep 17 00:00:00 2001 From: Benjamin Sergeant Date: Sat, 11 May 2019 11:45:20 -0700 Subject: [PATCH] fix race condition in SelectInteruptPipe, where _fildes are not protected (caught by fedora tsan) --- ixwebsocket/IXSelectInterruptPipe.cpp | 8 ++++++++ ixwebsocket/IXSelectInterruptPipe.h | 2 ++ ixwebsocket/IXWebSocket.cpp | 7 ------- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/ixwebsocket/IXSelectInterruptPipe.cpp b/ixwebsocket/IXSelectInterruptPipe.cpp index 5444cbe3..9d27bbd5 100644 --- a/ixwebsocket/IXSelectInterruptPipe.cpp +++ b/ixwebsocket/IXSelectInterruptPipe.cpp @@ -40,6 +40,8 @@ namespace ix bool SelectInterruptPipe::init(std::string& errorMsg) { + std::lock_guard lock(_fildesMutex); + // calling init twice is a programming error assert(_fildes[kPipeReadIndex] == -1); assert(_fildes[kPipeWriteIndex] == -1); @@ -108,6 +110,8 @@ namespace ix bool SelectInterruptPipe::notify(uint64_t value) { + std::lock_guard lock(_fildesMutex); + int fd = _fildes[kPipeWriteIndex]; if (fd == -1) return false; @@ -118,6 +122,8 @@ namespace ix // TODO: return max uint64_t for errors ? uint64_t SelectInterruptPipe::read() { + std::lock_guard lock(_fildesMutex); + int fd = _fildes[kPipeReadIndex]; uint64_t value = 0; @@ -133,6 +139,8 @@ namespace ix int SelectInterruptPipe::getFd() const { + std::lock_guard lock(_fildesMutex); + return _fildes[kPipeReadIndex]; } } diff --git a/ixwebsocket/IXSelectInterruptPipe.h b/ixwebsocket/IXSelectInterruptPipe.h index 8c0a4df4..1240f9a5 100644 --- a/ixwebsocket/IXSelectInterruptPipe.h +++ b/ixwebsocket/IXSelectInterruptPipe.h @@ -10,6 +10,7 @@ #include #include +#include namespace ix { @@ -30,6 +31,7 @@ namespace ix // happens between a control thread and a background thread, which is // blocked on select. int _fildes[2]; + mutable std::mutex _fildesMutex; // Used to identify the read/write idx static const int kPipeReadIndex; diff --git a/ixwebsocket/IXWebSocket.cpp b/ixwebsocket/IXWebSocket.cpp index 84cc3dc8..cd349827 100644 --- a/ixwebsocket/IXWebSocket.cpp +++ b/ixwebsocket/IXWebSocket.cpp @@ -144,11 +144,6 @@ namespace ix void WebSocket::stop() { - bool automaticReconnection = _automaticReconnection; - - // This value needs to be forced when shutting down, it is restored later - _automaticReconnection = false; - close(); if (_thread.joinable()) @@ -157,8 +152,6 @@ namespace ix _thread.join(); _stop = false; } - - _automaticReconnection = automaticReconnection; } WebSocketInitResult WebSocket::connect(int timeoutSecs)