fix race condition in SelectInteruptPipe, where _fildes are not protected (caught by fedora tsan)
This commit is contained in:
		@@ -40,6 +40,8 @@ namespace ix
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    bool SelectInterruptPipe::init(std::string& errorMsg)
 | 
					    bool SelectInterruptPipe::init(std::string& errorMsg)
 | 
				
			||||||
    {
 | 
					    {
 | 
				
			||||||
 | 
					        std::lock_guard<std::mutex> lock(_fildesMutex);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        // calling init twice is a programming error
 | 
					        // calling init twice is a programming error
 | 
				
			||||||
        assert(_fildes[kPipeReadIndex] == -1);
 | 
					        assert(_fildes[kPipeReadIndex] == -1);
 | 
				
			||||||
        assert(_fildes[kPipeWriteIndex] == -1);
 | 
					        assert(_fildes[kPipeWriteIndex] == -1);
 | 
				
			||||||
@@ -108,6 +110,8 @@ namespace ix
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    bool SelectInterruptPipe::notify(uint64_t value)
 | 
					    bool SelectInterruptPipe::notify(uint64_t value)
 | 
				
			||||||
    {
 | 
					    {
 | 
				
			||||||
 | 
					        std::lock_guard<std::mutex> lock(_fildesMutex);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        int fd = _fildes[kPipeWriteIndex];
 | 
					        int fd = _fildes[kPipeWriteIndex];
 | 
				
			||||||
        if (fd == -1) return false;
 | 
					        if (fd == -1) return false;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -118,6 +122,8 @@ namespace ix
 | 
				
			|||||||
    // TODO: return max uint64_t for errors ?
 | 
					    // TODO: return max uint64_t for errors ?
 | 
				
			||||||
    uint64_t SelectInterruptPipe::read()
 | 
					    uint64_t SelectInterruptPipe::read()
 | 
				
			||||||
    {
 | 
					    {
 | 
				
			||||||
 | 
					        std::lock_guard<std::mutex> lock(_fildesMutex);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        int fd = _fildes[kPipeReadIndex];
 | 
					        int fd = _fildes[kPipeReadIndex];
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        uint64_t value = 0;
 | 
					        uint64_t value = 0;
 | 
				
			||||||
@@ -133,6 +139,8 @@ namespace ix
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    int SelectInterruptPipe::getFd() const
 | 
					    int SelectInterruptPipe::getFd() const
 | 
				
			||||||
    {
 | 
					    {
 | 
				
			||||||
 | 
					        std::lock_guard<std::mutex> lock(_fildesMutex);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        return _fildes[kPipeReadIndex];
 | 
					        return _fildes[kPipeReadIndex];
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -10,6 +10,7 @@
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
#include <stdint.h>
 | 
					#include <stdint.h>
 | 
				
			||||||
#include <string>
 | 
					#include <string>
 | 
				
			||||||
 | 
					#include <mutex>
 | 
				
			||||||
 | 
					
 | 
				
			||||||
namespace ix
 | 
					namespace ix
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
@@ -30,6 +31,7 @@ namespace ix
 | 
				
			|||||||
        // happens between a control thread and a background thread, which is
 | 
					        // happens between a control thread and a background thread, which is
 | 
				
			||||||
        // blocked on select.
 | 
					        // blocked on select.
 | 
				
			||||||
        int _fildes[2];
 | 
					        int _fildes[2];
 | 
				
			||||||
 | 
					        mutable std::mutex _fildesMutex;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        // Used to identify the read/write idx
 | 
					        // Used to identify the read/write idx
 | 
				
			||||||
        static const int kPipeReadIndex;
 | 
					        static const int kPipeReadIndex;
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -144,11 +144,6 @@ namespace ix
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    void WebSocket::stop()
 | 
					    void WebSocket::stop()
 | 
				
			||||||
    {
 | 
					    {
 | 
				
			||||||
        bool automaticReconnection = _automaticReconnection;
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
        // This value needs to be forced when shutting down, it is restored later
 | 
					 | 
				
			||||||
        _automaticReconnection = false;
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
        close();
 | 
					        close();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        if (_thread.joinable())
 | 
					        if (_thread.joinable())
 | 
				
			||||||
@@ -157,8 +152,6 @@ namespace ix
 | 
				
			|||||||
            _thread.join();
 | 
					            _thread.join();
 | 
				
			||||||
            _stop = false;
 | 
					            _stop = false;
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
 | 
					 | 
				
			||||||
        _automaticReconnection = automaticReconnection;
 | 
					 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    WebSocketInitResult WebSocket::connect(int timeoutSecs)
 | 
					    WebSocketInitResult WebSocket::connect(int timeoutSecs)
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user