From df21920b800cf87bce324de8a2f2aef878fbbf80 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 2 Jan 2009 21:53:30 +0000 Subject: [PATCH] disable the events when we get a notification about socket being ready for IO and reenable them later after performing the IO in the Unix version to avoid continuous flood of ready notifications git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@57796 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/msw/private/sockmsw.h | 8 ++ include/wx/private/socket.h | 5 + include/wx/unix/private/sockunix.h | 30 +++++- src/common/socket.cpp | 25 ++--- src/unix/sockunix.cpp | 160 +++++++++++++++++------------ 5 files changed, 140 insertions(+), 88 deletions(-) diff --git a/include/wx/msw/private/sockmsw.h b/include/wx/msw/private/sockmsw.h index 1c0a6fe627..5313b5446e 100644 --- a/include/wx/msw/private/sockmsw.h +++ b/include/wx/msw/private/sockmsw.h @@ -39,6 +39,14 @@ public: virtual wxSocketError GetLastError() const; + virtual void ReenableEvents(wxSocketEventFlags WXUNUSED(flags)) + { + // notifications are never disabled in this implementation, there is no + // need for this as WSAAsyncSelect() only sends notification once when + // the new data becomes available anyhow, so there is no need to do + // anything here + } + private: virtual void DoClose(); diff --git a/include/wx/private/socket.h b/include/wx/private/socket.h index 0bc2f3b810..c362324b2c 100644 --- a/include/wx/private/socket.h +++ b/include/wx/private/socket.h @@ -286,6 +286,11 @@ public: // named) OnRequest() method void NotifyOnStateChange(wxSocketNotify event); + // called after reading/writing the data from/to the socket and should + // enable back the wxSOCKET_INPUT/OUTPUT_FLAG notifications if they were + // turned off when this data was first detected + virtual void ReenableEvents(wxSocketEventFlags flags) = 0; + // TODO: make these fields protected and provide accessors for those of // them that wxSocketBase really needs //protected: diff --git a/include/wx/unix/private/sockunix.h b/include/wx/unix/private/sockunix.h index afdcc4e0cf..2531060364 100644 --- a/include/wx/unix/private/sockunix.h +++ b/include/wx/unix/private/sockunix.h @@ -31,6 +31,23 @@ public: virtual wxSocketError GetLastError() const; + virtual void ReenableEvents(wxSocketEventFlags flags) + { + // enable the notifications about input/output being available again in + // case they were disabled by OnRead/WriteWaiting() + // + // notice that we'd like to enable the events here only if there is + // nothing more left on the socket right now as otherwise we're going + // to get a "ready for whatever" notification immediately (well, during + // the next event loop iteration) and disable the event back again + // which is rather inefficient but unfortunately doing it like this + // doesn't work because the existing code (e.g. src/common/sckipc.cpp) + // expects to keep getting notifications about the data available from + // the socket even if it didn't read all the data the last time, so we + // absolutely have to continue generating them + EnableEvents(flags); + } + // wxFDIOHandler methods virtual void OnReadWaiting(); virtual void OnWriteWaiting(); @@ -61,11 +78,13 @@ private: } // enable or disable notifications for socket input/output events - void EnableEvents() { DoEnableEvents(true); } - void DisableEvents() { DoEnableEvents(false); } + void EnableEvents(int flags = wxSOCKET_INPUT_FLAG | wxSOCKET_OUTPUT_FLAG) + { DoEnableEvents(flags, true); } + void DisableEvents(int flags = wxSOCKET_INPUT_FLAG | wxSOCKET_OUTPUT_FLAG) + { DoEnableEvents(flags, false); } // really enable or disable socket input/output events - void DoEnableEvents(bool enable); + void DoEnableEvents(int flags, bool enable); protected: // descriptors for input and output event notification channels associated @@ -81,6 +100,11 @@ private: // down the socket if the event is wxSOCKET_LOST void OnStateChange(wxSocketNotify event); + // check if there is any input available, return 1 if yes, 0 if no or -1 on + // error + int CheckForInput(); + + // give it access to our m_fds friend class wxSocketFDBasedManager; }; diff --git a/src/common/socket.cpp b/src/common/socket.cpp index 18de1040e6..5b8339c92d 100644 --- a/src/common/socket.cpp +++ b/src/common/socket.cpp @@ -176,6 +176,8 @@ public: ~wxSocketReadGuard() { m_socket->m_reading = false; + + m_socket->m_impl->ReenableEvents(wxSOCKET_INPUT_FLAG); } private: @@ -193,6 +195,8 @@ public: wxASSERT_MSG( !m_socket->m_writing, "write reentrancy?" ); m_socket->m_writing = true; + + m_socket->m_impl->ReenableEvents(wxSOCKET_OUTPUT_FLAG); } ~wxSocketWriteGuard() @@ -472,6 +476,10 @@ wxSocketImpl *wxSocketImpl::Accept(wxSocketBase& wxsocket) WX_SOCKLEN_T fromlen = sizeof(from); const SOCKET fd = accept(m_fd, &from.addr, &fromlen); + // accepting is similar to reading in the sense that it resets "ready for + // read" flag on the socket + ReenableEvents(wxSOCKET_INPUT_FLAG); + if ( fd == INVALID_SOCKET ) return NULL; @@ -494,10 +502,6 @@ void wxSocketImpl::Close() } } -/* - * Disallow further read/write operations on this socket, close - * the fd and disable all callbacks. - */ void wxSocketImpl::Shutdown() { if ( m_fd != INVALID_SOCKET ) @@ -1568,19 +1572,6 @@ void wxSocketBase::OnRequest(wxSocketNotify notification) // send the wx event if enabled and we're interested in it if ( m_notify && (m_eventmask & flag) && m_handler ) { - // If we are in the middle of a R/W operation, do not propagate events - // to users. Also, filter 'late' events which are no longer valid. - if ( notification == wxSOCKET_INPUT ) - { - if ( m_reading || !m_impl->Select(wxSOCKET_INPUT_FLAG) ) - return; - } - else if ( notification == wxSOCKET_OUTPUT ) - { - if ( m_writing || !m_impl->Select(wxSOCKET_OUTPUT_FLAG) ) - return; - } - wxSocketEvent event(m_id); event.m_event = notification; event.m_clientData = m_clientData; diff --git a/src/unix/sockunix.cpp b/src/unix/sockunix.cpp index 6383b54ac5..c50debc326 100644 --- a/src/unix/sockunix.cpp +++ b/src/unix/sockunix.cpp @@ -104,21 +104,36 @@ wxSocketError wxSocketImplUnix::GetLastError() const } } -void wxSocketImplUnix::DoEnableEvents(bool flag) +void wxSocketImplUnix::DoEnableEvents(int flags, bool enable) { wxSocketManager * const manager = wxSocketManager::Get(); - if ( flag ) + if ( enable ) { - manager->Install_Callback(this, wxSOCKET_INPUT); - manager->Install_Callback(this, wxSOCKET_OUTPUT); + if ( flags & wxSOCKET_INPUT_FLAG ) + manager->Install_Callback(this, wxSOCKET_INPUT); + if ( flags & wxSOCKET_OUTPUT_FLAG ) + manager->Install_Callback(this, wxSOCKET_OUTPUT); } else // off { - manager->Uninstall_Callback(this, wxSOCKET_INPUT); - manager->Uninstall_Callback(this, wxSOCKET_OUTPUT); + if ( flags & wxSOCKET_INPUT_FLAG ) + manager->Uninstall_Callback(this, wxSOCKET_INPUT); + if ( flags & wxSOCKET_OUTPUT_FLAG ) + manager->Uninstall_Callback(this, wxSOCKET_OUTPUT); } } +int wxSocketImplUnix::CheckForInput() +{ + char c; + int rc; + do + { + rc = recv(m_fd, &c, 1, MSG_PEEK); + } while ( rc == -1 && errno == EINTR ); + + return rc; +} void wxSocketImplUnix::OnStateChange(wxSocketNotify event) { @@ -130,82 +145,91 @@ void wxSocketImplUnix::OnStateChange(wxSocketNotify event) void wxSocketImplUnix::OnReadWaiting() { - char c; - - if (m_fd == INVALID_SOCKET) - { - return; - } - - int num = recv(m_fd, &c, 1, MSG_PEEK); - - if (num > 0) - { - OnStateChange(wxSOCKET_INPUT); - } - else - { - if (m_server && m_stream) - { - OnStateChange(wxSOCKET_CONNECTION); - } - else if (num == 0) + wxASSERT_MSG( m_fd != INVALID_SOCKET, "invalid socket ready for reading?" ); + + // we need to disable the read notifications until we read all the data + // already available for the socket, otherwise we're going to keep getting + // them continuously which is worse than inefficient: as IO notifications + // have higher priority than idle events in e.g. GTK+, our pending events + // whose handlers typically call Read() which would consume the data and so + // stop the notifications flood would never be dispatched at all if the + // notifications were not disabled + DisableEvents(wxSOCKET_INPUT_FLAG); + + + // find out what are we going to notify about exactly + wxSocketNotify notify; + + // TCP listening sockets become ready for reading when there is a pending + // connection + if ( m_server && m_stream ) { - if (m_stream) - { - /* graceful shutdown */ - OnStateChange(wxSOCKET_LOST); - } - else - { - /* Empty datagram received */ - OnStateChange(wxSOCKET_INPUT); - } + notify = wxSOCKET_CONNECTION; } - else + else // check if there is really any input available { - /* Do not throw a lost event in cases where the socket isn't really lost */ - if ((errno == EWOULDBLOCK) || (errno == EAGAIN) || (errno == EINTR)) - { - OnStateChange(wxSOCKET_INPUT); - } - else - { - OnStateChange(wxSOCKET_LOST); - } + switch ( CheckForInput() ) + { + case 1: + notify = wxSOCKET_INPUT; + break; + + case 0: + // reading 0 bytes for a TCP socket means that the connection + // was closed by peer but for UDP it just means that we got an + // empty datagram + notify = m_stream ? wxSOCKET_LOST : wxSOCKET_INPUT; + break; + + default: + wxFAIL_MSG( "unexpected CheckForInput() return value" ); + // fall through + + case -1: + if ( GetLastError() == wxSOCKET_WOULDBLOCK ) + { + // just a spurious wake up + EnableEvents(wxSOCKET_INPUT_FLAG); + return; + } + + notify = wxSOCKET_LOST; + } } - } + + OnStateChange(notify); } void wxSocketImplUnix::OnWriteWaiting() { - if (m_establishing && !m_server) - { - int error; - SOCKOPTLEN_T len = sizeof(error); + wxASSERT_MSG( m_fd != INVALID_SOCKET, "invalid socket ready for writing?" ); - m_establishing = false; + // see comment in the beginning of OnReadWaiting() above + DisableEvents(wxSOCKET_OUTPUT_FLAG); - getsockopt(m_fd, SOL_SOCKET, SO_ERROR, (char*)&error, &len); - if (error) + // check whether this is a notification for the completion of a + // non-blocking connect() + if ( m_establishing && !m_server ) { - OnStateChange(wxSOCKET_LOST); - } - else - { - OnStateChange(wxSOCKET_CONNECTION); - /* We have to fire this event by hand because CONNECTION (for clients) - * and OUTPUT are internally the same and we just disabled CONNECTION - * events with the above macro. - */ - OnStateChange(wxSOCKET_OUTPUT); + m_establishing = false; + + // check whether we connected successfully + int error; + SOCKOPTLEN_T len = sizeof(error); + + getsockopt(m_fd, SOL_SOCKET, SO_ERROR, (char*)&error, &len); + + if ( error ) + { + OnStateChange(wxSOCKET_LOST); + return; + } + + OnStateChange(wxSOCKET_CONNECTION); } - } - else - { + OnStateChange(wxSOCKET_OUTPUT); - } } void wxSocketImplUnix::OnExceptionWaiting() -- 2.45.2