From dadb7b9fb0ae63d0ef7cc66c6433e22c95ee240b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 15 Aug 2017 14:22:43 +0200 Subject: [PATCH] Don't register async notifications for MSW blocking sockets Under MSW calling UnblockAndRegisterWithEventLoop() for blocking sockets is not only useless, but actually harmful when the socket is used from a worker thread (which is the common case for blocking sockets), as it means that the main thread will be receiving notifications for the socket events and modifying the socket object while it's being used from the other thread, resulting in data races and general brokenness. This is similar to e18c8fd29a10b1b87ea5cff7c5b3a16fe5b32690 for Unix sockets. Closes #17937. --- include/wx/msw/private/sockmsw.h | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/include/wx/msw/private/sockmsw.h b/include/wx/msw/private/sockmsw.h index 5c4e565d20..bbcad77bca 100644 --- a/include/wx/msw/private/sockmsw.h +++ b/include/wx/msw/private/sockmsw.h @@ -51,9 +51,25 @@ private: virtual void UnblockAndRegisterWithEventLoop() wxOVERRIDE { - // no need to make the socket non-blocking, Install_Callback() will do - // it - wxSocketManager::Get()->Install_Callback(this); + if ( GetSocketFlags() & wxSOCKET_BLOCK ) + { + // Counter-intuitively, we make the socket non-blocking even in + // this case as it is necessary e.g. for Read() to return + // immediately if there is no data available. However we must not + // install a callback for it as blocking sockets don't use any + // events and generating them would actually be harmful (and not + // just useless) as they would be dispatched by the main thread + // while this blocking socket can be used from a worker one, so it + // would result in data races and other unpleasantness. + unsigned long trueArg = 1; + ioctlsocket(m_fd, FIONBIO, &trueArg); + } + else + { + // No need to make the socket non-blocking, Install_Callback() will + // do it as a side effect of calling WSAAsyncSelect(). + wxSocketManager::Get()->Install_Callback(this); + } } int m_msgnumber;