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; diff --git a/include/wx/socket.h b/include/wx/socket.h index daded4bf05..4a66716984 100644 --- a/include/wx/socket.h +++ b/include/wx/socket.h @@ -225,6 +225,16 @@ public: bool IsNoWait() const { return ((m_flags & wxSOCKET_NOWAIT) != 0); } wxSocketType GetType() const { return m_type; } + // Helper returning wxSOCKET_NONE if non-blocking sockets can be used, i.e. + // the socket is being created in the main thread and the event loop is + // running, or wxSOCKET_BLOCK otherwise. + // + // This is an internal function used only by wxWidgets itself, user code + // should decide if it wants blocking sockets or not and use the + // appropriate style instead of using it (but wxWidgets has to do it like + // this for compatibility with the original network classes behaviour). + static int GetBlockingFlagIfNeeded(); + private: friend class wxSocketClient; friend class wxSocketServer; diff --git a/interface/wx/socket.h b/interface/wx/socket.h index 0de0c81e43..36ad0f87d2 100644 --- a/interface/wx/socket.h +++ b/interface/wx/socket.h @@ -289,6 +289,12 @@ public: /** Constructor. + Notice that if the object is created from a worker thread or if it is + created from the main thread but the event loop is not running, @a + flags parameter @em must include ::wxSOCKET_BLOCK as non-blocking + sockets require dispatching events, which can only be done in the main + thread. + @param flags Socket flags (See wxSocketBase::SetFlags()) */ diff --git a/src/common/ftp.cpp b/src/common/ftp.cpp index 4644099901..d1cd21b60b 100644 --- a/src/common/ftp.cpp +++ b/src/common/ftp.cpp @@ -582,7 +582,13 @@ wxSocketBase *wxFTP::GetActivePort() addrNew.AnyAddress(); addrNew.Service(0); // pick an open port number. - wxSocketServer *sockSrv = new wxSocketServer(addrNew); + wxSocketServer* const + sockSrv = new wxSocketServer + ( + addrNew, + wxSocketServer::GetBlockingFlagIfNeeded() + ); + if (!sockSrv->IsOk()) { // We use IsOk() here to see if everything is ok @@ -647,7 +653,11 @@ wxSocketBase *wxFTP::GetPassivePort() addr.Hostname(hostaddr); addr.Service(port); - wxSocketClient *client = new wxSocketClient(); + // If we're used from a worker thread or can't dispatch events even though + // we're in the main one, we can't use non-blocking sockets. + wxSocketClient* const + client = new wxSocketClient(wxSocketClient::GetBlockingFlagIfNeeded()); + if ( !client->Connect(addr) ) { m_lastError = wxPROTO_CONNERR; diff --git a/src/common/protocol.cpp b/src/common/protocol.cpp index f6250252c3..ffe0e0f086 100644 --- a/src/common/protocol.cpp +++ b/src/common/protocol.cpp @@ -65,9 +65,7 @@ wxIMPLEMENT_ABSTRACT_CLASS(wxProtocol, wxObject); wxProtocol::wxProtocol() #if wxUSE_SOCKETS // Only use non blocking sockets if we can dispatch events. - : wxSocketClient((wxIsMainThread() && wxApp::IsMainLoopRunning() - ? wxSOCKET_NONE - : wxSOCKET_BLOCK) | wxSOCKET_WAITALL) + : wxSocketClient(wxSocketClient::GetBlockingFlagIfNeeded() | wxSOCKET_WAITALL) #endif { m_lastError = wxPROTO_NOERR; diff --git a/src/common/socket.cpp b/src/common/socket.cpp index 24214b1859..5fb8c291ae 100644 --- a/src/common/socket.cpp +++ b/src/common/socket.cpp @@ -928,6 +928,14 @@ wxSocketError wxSocketBase::LastError() const return m_impl->GetError(); } +/* static */ +int wxSocketBase::GetBlockingFlagIfNeeded() +{ + return wxIsMainThread() && wxApp::IsMainLoopRunning() + ? wxSOCKET_NONE + : wxSOCKET_BLOCK; +} + // -------------------------------------------------------------------------- // Basic IO calls // -------------------------------------------------------------------------- @@ -1966,6 +1974,15 @@ bool wxSocketBase::SetLocal(const wxIPV4address& local) wxSocketClient::wxSocketClient(wxSocketFlags flags) : wxSocketBase(flags, wxSOCKET_CLIENT) { + // Notice that we don't check for a running event loop here, unlike in + // GetBlockingFlagIfNeeded() because it is common to create the sockets + // before the event loop is entered and we shouldn't break existing code + // doing this as it can still work correctly if it only uses non-blocking + // sockets once the event loop is running. + wxASSERT_MSG( (flags & wxSOCKET_BLOCK) || wxIsMainThread(), + wxS("Non-blocking sockets may only be created ") + wxS("in the main thread") ); + m_initialRecvBufferSize = m_initialSendBufferSize = -1; } diff --git a/src/msw/sockmsw.cpp b/src/msw/sockmsw.cpp index 2330a33a6c..791f18a49b 100644 --- a/src/msw/sockmsw.cpp +++ b/src/msw/sockmsw.cpp @@ -267,15 +267,12 @@ void wxSocketMSWManager::Install_Callback(wxSocketImpl *socket_, { wxSocketImplMSW * const socket = static_cast(socket_); - if (socket->m_fd != INVALID_SOCKET) - { /* We could probably just subscribe to all events regardless * of the socket type, but MS recommends to do it this way. */ long lEvent = socket->m_server? FD_ACCEPT : (FD_READ | FD_WRITE | FD_CONNECT | FD_CLOSE); gs_WSAAsyncSelect(socket->m_fd, hWin, socket->m_msgnumber, lEvent); - } } /* @@ -286,10 +283,7 @@ void wxSocketMSWManager::Uninstall_Callback(wxSocketImpl *socket_, { wxSocketImplMSW * const socket = static_cast(socket_); - if (socket->m_fd != INVALID_SOCKET) - { gs_WSAAsyncSelect(socket->m_fd, hWin, socket->m_msgnumber, 0); - } } // set the wxBase variable to point to our wxSocketManager implementation