Merge branch 'sock-event-fix'

Fix unwanted (and sometimes fatal) socket events for blocking sockets
under Unix.

See https://github.com/wxWidgets/wxWidgets/pull/1658

include/wx/msw/private/sockmsw.h

@@ -57,10 +57,7 @@ public:
         // anything here
     }
 
-private:
+    virtual void UpdateBlockingState() wxOVERRIDE
-    virtual void DoClose() wxOVERRIDE;
-
-    virtual void UnblockAndRegisterWithEventLoop() wxOVERRIDE
     {
         if ( GetSocketFlags() & wxSOCKET_BLOCK )
         {
@@ -74,6 +71,9 @@ private:
             // would result in data races and other unpleasantness.
             wxIoctlSocketArg_t trueArg = 1;
             ioctlsocket(m_fd, FIONBIO, &trueArg);
+
+            // Uninstall it in case it was installed before.
+            wxSocketManager::Get()->Uninstall_Callback(this);
         }
         else
         {
@@ -83,6 +83,9 @@ private:
         }
     }
 
+private:
+    virtual void DoClose() wxOVERRIDE;
+
     int m_msgnumber;
 
     friend class wxSocketMSWManager;

include/wx/private/socket.h

@@ -281,6 +281,12 @@ public:
     // notifications
     // -------------
 
+    // Update the socket depending on the presence or absence of wxSOCKET_BLOCK
+    // in GetSocketFlags(): if it's present, make the socket blocking and
+    // ensure that we don't get any asynchronous event for it, otherwise put
+    // it into non-blocking mode and enable monitoring it in the event loop.
+    virtual void UpdateBlockingState() = 0;
+
     // notify m_wxsocket about the given socket event by calling its (inaptly
     // named) OnRequest() method
     void NotifyOnStateChange(wxSocketNotify event);
@@ -323,10 +329,6 @@ private:
     // called by Close() if we have a valid m_fd
     virtual void DoClose() = 0;
 
-    // put this socket into non-blocking mode and enable monitoring this socket
-    // as part of the event loop
-    virtual void UnblockAndRegisterWithEventLoop() = 0;
-
     // check that the socket wasn't created yet and that the given address
     // (either m_local or m_peer depending on the socket kind) is valid and
     // set m_error and return false if this is not the case
@@ -350,7 +352,7 @@ private:
     }
 
     // apply the options to the (just created) socket and register it with the
-    // event loop by calling UnblockAndRegisterWithEventLoop()
+    // event loop by calling UpdateBlockingState()
     void PostCreation();
 
     // update local address after binding/connecting

include/wx/unix/private/sockunix.h

@@ -40,6 +40,10 @@ public:
 
     virtual void ReenableEvents(wxSocketEventFlags flags) wxOVERRIDE
     {
+        // Events are only ever used for non-blocking sockets.
+        if ( GetSocketFlags() & wxSOCKET_BLOCK )
+            return;
+
         // enable the notifications about input/output being available again in
         // case they were disabled by OnRead/WriteWaiting()
         //
@@ -55,6 +59,15 @@ public:
         EnableEvents(flags);
     }
 
+    virtual void UpdateBlockingState() wxOVERRIDE
+    {
+        // Make this int and not bool to allow passing it to ioctl().
+        const int isBlocking = (GetSocketFlags() & wxSOCKET_BLOCK) != 0;
+        ioctl(m_fd, FIONBIO, &isBlocking);
+
+        DoEnableEvents(wxSOCKET_INPUT_FLAG | wxSOCKET_OUTPUT_FLAG, !isBlocking);
+    }
+
     // wxFDIOHandler methods
     virtual void OnReadWaiting() wxOVERRIDE;
     virtual void OnWriteWaiting() wxOVERRIDE;
@@ -69,14 +82,6 @@ private:
         wxCloseSocket(m_fd);
     }
 
-    virtual void UnblockAndRegisterWithEventLoop() wxOVERRIDE
-    {
-        int trueArg = 1;
-        ioctl(m_fd, FIONBIO, &trueArg);
-
-        EnableEvents();
-    }
-
     // enable or disable notifications for socket input/output events
     void EnableEvents(int flags = wxSOCKET_INPUT_FLAG | wxSOCKET_OUTPUT_FLAG)
         { DoEnableEvents(flags, true); }

src/common/socket.cpp

@@ -366,9 +366,9 @@ void wxSocketImpl::PostCreation()
     if ( m_initialSendBufferSize >= 0 )
         SetSocketOption(SO_SNDBUF, m_initialSendBufferSize);
 
-    // we always put our sockets in unblocked mode and handle blocking
+    // Call this to put our socket in unblocked mode: we'll handle blocking
     // ourselves in DoRead/Write() if wxSOCKET_WAITALL is specified
-    UnblockAndRegisterWithEventLoop();
+    UpdateBlockingState();
 }
 
 wxSocketError wxSocketImpl::UpdateLocalAddress()
@@ -551,7 +551,7 @@ wxSocketImpl *wxSocketImpl::Accept(wxSocketBase& wxsocket)
     sock->m_fd = fd;
     sock->m_peer = wxSockAddressImpl(from.addr, fromlen);
 
-    sock->UnblockAndRegisterWithEventLoop();
+    sock->UpdateBlockingState();
 
     return sock;
 }
@@ -1672,7 +1672,20 @@ void wxSocketBase::SetFlags(wxSocketFlags flags)
                   "Using wxSOCKET_WAITALL or wxSOCKET_BLOCK with "
                   "wxSOCKET_NOWAIT doesn't make sense" );
 
+    // Blocking sockets are very different from non-blocking ones and we need
+    // to [un]register the socket with the event loop if wxSOCKET_BLOCK is
+    // being [un]set.
+    const bool
+        blockChanged = (m_flags & wxSOCKET_BLOCK) != (flags & wxSOCKET_BLOCK);
+
     m_flags = flags;
+
+    if ( blockChanged )
+    {
+        // Of course, we only do this if we already have the actual socket.
+        if ( m_impl )
+            m_impl->UpdateBlockingState();
+    }
 }
 
 

src/unix/sockunix.cpp

@@ -90,17 +90,18 @@ wxSocketError wxSocketImplUnix::GetLastError() const
 
 void wxSocketImplUnix::DoEnableEvents(int flags, bool enable)
 {
-    // No events for blocking sockets, they should be usable from the other
-    // threads and the events only work for the sockets used by the main one.
-    if ( GetSocketFlags() & wxSOCKET_BLOCK )
-        return;
-
     wxSocketManager * const manager = wxSocketManager::Get();
     if (!manager)
         return;
 
     if ( enable )
     {
+        // We should never try to enable events for the blocking sockets, they
+        // should be usable from the other threads and the events only work for
+        // the sockets used by the main one.
+        wxASSERT_MSG( !(GetSocketFlags() & wxSOCKET_BLOCK),
+                      "enabling events for a blocking socket?" );
+
         if ( flags & wxSOCKET_INPUT_FLAG )
             manager->Install_Callback(this, wxSOCKET_INPUT);
         if ( flags & wxSOCKET_OUTPUT_FLAG )