From 73f0c9dff8b84f22f799d2fb68f793eef4fc36bc Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Wed, 20 Nov 2019 19:00:30 +0100
Subject: [PATCH] Fix crash with blocking accepting sockets in threads under
 Unix

Sockets returned by wxSocket::Accept() are non-blocking by default and
the only way to use them safely in worker threads is by switching them
to the blocking mode by calling SetFlags(wxSOCKET_BLOCK).

However this didn't work correctly since at least 2.8 days, as turning
wxSOCKET_BLOCK on didn't unregister the socket from the event loop, with
which it had been registered on creation. Fix this by doing this now,
which ensures that the main thread doesn't get any notifications about
the socket if it's used, in a blocking way, in a worker thread.

Note that making the new socket blocking after accpeting is still pretty
inefficient and pre-creating the socket as blocking and using
AcceptWith() is still preferable, but at least it does work now.

Closes #12886.
---
 include/wx/msw/private/sockmsw.h   |  6 +++---
 include/wx/private/socket.h        | 12 ++++++------
 include/wx/unix/private/sockunix.h | 18 +++++++++---------
 src/common/socket.cpp              | 13 +++++++++++++
 4 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/include/wx/msw/private/sockmsw.h b/include/wx/msw/private/sockmsw.h
index 70844e4742..d6010fbc1b 100644
--- a/include/wx/msw/private/sockmsw.h
+++ b/include/wx/msw/private/sockmsw.h
@@ -57,9 +57,6 @@ public:
         // anything here
     }
 
-private:
-    virtual void DoClose() wxOVERRIDE;
-
     virtual void UpdateBlockingState() wxOVERRIDE
     {
         if ( GetSocketFlags() & wxSOCKET_BLOCK )
@@ -86,6 +83,9 @@ private:
         }
     }
 
+private:
+    virtual void DoClose() wxOVERRIDE;
+
     int m_msgnumber;
 
     friend class wxSocketMSWManager;
diff --git a/include/wx/private/socket.h b/include/wx/private/socket.h
index 2d184acc07..20db91c7e7 100644
--- a/include/wx/private/socket.h
+++ b/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,12 +329,6 @@ private:
     // called by Close() if we have a valid m_fd
     virtual void DoClose() = 0;
 
-    // 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;
-
     // 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
diff --git a/include/wx/unix/private/sockunix.h b/include/wx/unix/private/sockunix.h
index 8b3fc2bbb0..f07ecafd87 100644
--- a/include/wx/unix/private/sockunix.h
+++ b/include/wx/unix/private/sockunix.h
@@ -59,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;
@@ -73,15 +82,6 @@ private:
         wxCloseSocket(m_fd);
     }
 
-    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);
-    }
-
     // enable or disable notifications for socket input/output events
     void EnableEvents(int flags = wxSOCKET_INPUT_FLAG | wxSOCKET_OUTPUT_FLAG)
         { DoEnableEvents(flags, true); }
diff --git a/src/common/socket.cpp b/src/common/socket.cpp
index 3ae69fadd1..f6a5f57fa6 100644
--- a/src/common/socket.cpp
+++ b/src/common/socket.cpp
@@ -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();
+    }
 }